New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script file version resolver #25

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
@developingchris
Contributor

developingchris commented Oct 20, 2011

We've discussed a previous implementation of the up folder versioner. Here is a simpler overload of the versionfile parameter.

I have a style pull request forthcoming that fixes the naming of the private variables.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

How do you feel about a special key passed to the versionFile? Something to the effect of /vf=up and when the version resolver runs it sees that and uses the highest number script it finds.

How do you feel about a special key passed to the versionFile? Something to the effect of /vf=up and when the version resolver runs it sees that and uses the highest number script it finds.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

What happens when someone uses timestamps?

What happens when someone uses timestamps?

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

I just noticed your code conventions are a little off.

RH code conventions:
Classes are PascalCase.
Everything else is lowercase (including namespaces).
No underscores for fields.
All multi-word methods, variables, and everything else are lowercase words separated by underscores (i.e here_is_a_method()).

I just noticed your code conventions are a little off.

RH code conventions:
Classes are PascalCase.
Everything else is lowercase (including namespaces).
No underscores for fields.
All multi-word methods, variables, and everything else are lowercase words separated by underscores (i.e here_is_a_method()).

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

I don't mind the special key since it gets me what I'm looking for. However, it does seem kind of strange that there isn't a version type specified but merely a different file type, that signals how the version is determined.

in the case of timestamps, you'd get larger numbers, but thats not necessarily a bad thing. Thats what I'm using in my project.

I'll clean up my code to conform better and move it to be a hidden switch inside the version file and send it back over.

I don't mind the special key since it gets me what I'm looking for. However, it does seem kind of strange that there isn't a version type specified but merely a different file type, that signals how the version is determined.

in the case of timestamps, you'd get larger numbers, but thats not necessarily a bad thing. Thats what I'm using in my project.

I'll clean up my code to conform better and move it to be a hidden switch inside the version file and send it back over.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

How do you feel about something that notes it's looking at .sql files to figure it out? UP_SQL ?

How do you feel about something that notes it's looking at .sql files to figure it out? UP_SQL ?

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

so what feels wrong is, if its xml its a path. If its a dll its a path. If its the up, its just a word being tested for what if passing in "up/*.sql" which is really what I want, is the test. Its more explicit, but it stays pretty true to the nature of the argument, a path.

I do see the beauty of letting it figure it out. Conventions rock. But I also think that once you've decided to override the convention, it should no longer be fuzzy about what you are telling it not to infer.

so what feels wrong is, if its xml its a path. If its a dll its a path. If its the up, its just a word being tested for what if passing in "up/*.sql" which is really what I want, is the test. Its more explicit, but it stays pretty true to the nature of the argument, a path.

I do see the beauty of letting it figure it out. Conventions rock. But I also think that once you've decided to override the convention, it should no longer be fuzzy about what you are telling it not to infer.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

That's beautiful. I was looking for the same thing, but hadn't yet thought of "*" - so simple, yet so intuitive.

You know "simple ain't easy" - that's why it's good to bounce ideas off of folks.

That's beautiful. I was looking for the same thing, but hadn't yet thought of "*" - so simple, yet so intuitive.

You know "simple ain't easy" - that's why it's good to bounce ideas off of folks.

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

Cool, I'll get it worked up. I like it. Glad we could bounce it through.

Cool, I'll get it worked up. I like it. Glad we could bounce it through.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

When RH was still vaporware we were trying to figure out how to do versioning... and one day the idea came to me to use the revision, just like we did code. Makes perfect sense and it was like "Duh!" for both me and Dru. No one else was doing, yet it was the thing that once you thought of it, you were like, of course. :D

When RH was still vaporware we were trying to figure out how to do versioning... and one day the idea came to me to use the revision, just like we did code. Makes perfect sense and it was like "Duh!" for both me and Dru. No one else was doing, yet it was the thing that once you thought of it, you were like, of course. :D

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

Revision takes on a different meaning when you think of it like git, instead of like subversion though(a hash of the contents, not linear increments). You can move forward in time, but still move to the same revision as a previous moment in time. Which doesn't lend itself to this kind of versioning.

I see the beauty in it, and if you could have your rev in version control be respected by msbuild, the world would be a different place. But I actually like my chocolate and peanut butter seperate on this one. Build version, database version, and source control version. Are not the same things. In that branches will collide with each other. 2 branches may be completely different build numbers, that depend on the same revision of the database, but don't have the same version control revision, simply by the fact that they have 1 setting in a config file about the path of a custom error page. As much as it sounds like I'm making a contrived what if. This is really how I work a lot, on very small feature branches, where build rev bumps a lot, database changes happen sparingly and ultimately after a pretty solid rebase, 30 changesets become maybe 3. So tying the database to the assemblies, to the way I use sourcecontrol, constrains my ability to use git and mercurial to their fullest power.

Revision takes on a different meaning when you think of it like git, instead of like subversion though(a hash of the contents, not linear increments). You can move forward in time, but still move to the same revision as a previous moment in time. Which doesn't lend itself to this kind of versioning.

I see the beauty in it, and if you could have your rev in version control be respected by msbuild, the world would be a different place. But I actually like my chocolate and peanut butter seperate on this one. Build version, database version, and source control version. Are not the same things. In that branches will collide with each other. 2 branches may be completely different build numbers, that depend on the same revision of the database, but don't have the same version control revision, simply by the fact that they have 1 setting in a config file about the path of a custom error page. As much as it sounds like I'm making a contrived what if. This is really how I work a lot, on very small feature branches, where build rev bumps a lot, database changes happen sparingly and ultimately after a pretty solid rebase, 30 changesets become maybe 3. So tying the database to the assemblies, to the way I use sourcecontrol, constrains my ability to use git and mercurial to their fullest power.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

this is not necessary.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

Add it anyway. The resolvers are specification criteria. Which means they decide to run or not.

Add it anyway. The resolvers are specification criteria. Which means they decide to run or not.

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

however, kind of unintuitively the last one wins, always so order in the list matters. in the case of the code that is there, the xml file, looses to the dll version, always.

however, kind of unintuitively the last one wins, always so order in the list matters. in the case of the code that is there, the xml file, looses to the dll version, always.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

Confused? I've never seen this behavior.

Confused? I've never seen this behavior.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

Yeah, sorry man, I don't think your statement is correct. When the file ends with .xml, the xml resolver meets the criteria, therefore it is executed. When the file ends in .dll, the dll resolvers is executed.

These are never run out of the context of the complex resolver. That's why there is no need for a branch here (https://github.com/developingchris/roundhouse/blob/master/product/roundhouse/runners/RoundhouseMigrationRunner.cs#L123), but maybe one should be put in for safeguarding.

Yeah, sorry man, I don't think your statement is correct. When the file ends with .xml, the xml resolver meets the criteria, therefore it is executed. When the file ends in .dll, the dll resolvers is executed.

These are never run out of the context of the complex resolver. That's why there is no need for a branch here (https://github.com/developingchris/roundhouse/blob/master/product/roundhouse/runners/RoundhouseMigrationRunner.cs#L123), but maybe one should be put in for safeguarding.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

I know the code isn't the greatest in the world, especially areas like the migration runner. Certain items are in there that way on purpose, others are being corrected.

I know the code isn't the greatest in the world, especially areas like the migration runner. Certain items are in there that way on purpose, others are being corrected.

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

it's by discipline it hasn't happened. I see where you were going. I like that I could compose a new versioner in. I agree, there are dirty parts. But I would love to help clean it up, as this project has a chance to be the best migrations framework I've seen. Especially with the integration of something like opendbdiff. Something I've been playing with to generate diffs so that developers can just pull from source, get to work and as they make changes mark change points as significant to the database and then have that history persisted.

it's by discipline it hasn't happened. I see where you were going. I like that I could compose a new versioner in. I agree, there are dirty parts. But I would love to help clean it up, as this project has a chance to be the best migrations framework I've seen. Especially with the integration of something like opendbdiff. Something I've been playing with to generate diffs so that developers can just pull from source, get to work and as they make changes mark change points as significant to the database and then have that history persisted.

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

Interesting....

Interesting....

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 15, 2011

You've probably seen refresh database? https://github.com/chucknorris/roundhouse/wiki/Roundhouserefreshdatabasefnh

I'm working on the EF equivalent.

You've probably seen refresh database? https://github.com/chucknorris/roundhouse/wiki/Roundhouserefreshdatabasefnh

I'm working on the EF equivalent.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

not necessary

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

not necessary, the version file is the determination. If I set version file to UP or some convention like that, it should clue the resolvers into this being a script file versioner

not necessary, the version file is the determination. If I set version file to UP or some convention like that, it should clue the resolvers into this being a script file versioner

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

none of this is necessary. I kind of went backwards through these commits. :D

none of this is necessary. I kind of went backwards through these commits. :D

This comment has been minimized.

Show comment
Hide comment
@developingchris

developingchris Oct 15, 2011

Owner

Well its unnecessary if we overload the vf setting

Well its unnecessary if we overload the vf setting

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

use the known folder for up. Pass that in because it already knows where that is.

use the known folder for up. Pass that in because it already knows where that is.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

doh, nevermind...my first comment if you got it.
This is not necessary. Pass in the VersionFile instead.

doh, nevermind...my first comment if you got it.
This is not necessary. Pass in the VersionFile instead.

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 14, 2011

This should be

return version_file.to_lower() == "up";

This should be

return version_file.to_lower() == "up";

making version resolver more straight forward to configure
reverted config changes that were overcomplicated
@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 20, 2011

Awesome... :D

Awesome... :D

@ferventcoder

This comment has been minimized.

Show comment
Hide comment
@ferventcoder

ferventcoder Oct 23, 2011

Member

This is fixed in 663f6c2

Member

ferventcoder commented Oct 23, 2011

This is fixed in 663f6c2

@ferventcoder ferventcoder modified the milestone: 0.8.5 Jun 3, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment