search

frioux / DBIx-Class-DeploymentHandler

https://metacpan.org/pod/DBIx::Class::DeploymentHandler
20 stars 25 forks source link

Separate install and deploy #54

Closed Altreus closed 5 years ago

Altreus commented 7 years ago

Further to my discussion https://github.com/frioux/DBIx-Class-DeploymentHandler/issues/50, I've fixed it up so that the change actually works. This was simply done by calling install_version_storage inside install and then ignoring __VERSION inside everything else.

Please note this also includes the commit found here https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/53 because I discovered that problem as a result of this amendment, so I had to have it in the branch.

Altreus commented 7 years ago

I've noticed that the ignore_ddl option causes it to try to create the __VERSION resultset (because it doesn't care about the filenames at this point).

I'm not sure whether I should be skipping that or not.

Altreus commented 7 years ago

This latest commit breaks the deprecated tests but I don't understand why. I've pushed it in the hopes that someone else has the experience to see it straight away.

Altreus commented 7 years ago

oh because we're not testing that that even happens, so my commit has actually removed behaviour

Altreus commented 7 years ago

OK sod that. The only way I can get deploy to save a version to the database is to rename deploy in HandlesDeploy and proxy to the new method.

The goal here was to separate it up so that install was install_version_storage and then deploy, allowing me to deploy separately. But now it's at a place where deploy correctly updates the database but does not write the version to version storage, and I can't work out how to do that bit automatically.

Sorry for the comment spam, but I'm half cataloguing my hacking process so when I come back to it I don't have to re-learn what I've discovered

Altreus commented 7 years ago

Can I make it one of the "ReasonableDefaults"? :thinking:

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.04%) to 92.415% when pulling 5469b93d2e795f3f717e766fdd03a0ab189c80c7 on Altreus:separate-install-and-deploy into c6317d8d31adccf6af6f473e0276276c969cf53f on frioux:master.

Altreus commented 7 years ago

By making it a "reasonable default" that a deployment adds a version to the database I seem to be maintaining existing functionality while also allowing my new module to work as expected.

https://github.com/Altreus/DBIx-Class-DeploymentHandler-VersionStorage-WithSchema

mohawk2 commented 5 years ago

@Altreus Could you bring the tests into a state where they pass? Merging is not going to happen without that. I may have follow-ups after a detailed reading of the related issues.

Altreus commented 5 years ago

Re #50, we don't need this any more and I don't think it would be friendly to everyone already using DH.