Skip to content
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

Adding new configuration-option "organize-versions" #307

Closed
wants to merge 4 commits into from
Closed

Adding new configuration-option "organize-versions" #307

wants to merge 4 commits into from

Conversation

SchulteMarkus
Copy link

This PR introduces an new configuration-option "organize-versions", which can be set to "year" or "year_and_month". If set, the versions-classes (files) will be in appropriate sub-directories relative to "migrations-directory". For example, if you set "organize-versions" to "year" in your doctrine-migration configuration, and generate a new version, the new file will be placed in migrations-directory/2015/ (if the actual year is 2015).

This change is use-case-driven. When using doctrine-migration, having every version (98 by now) in the "migrations-directory" became ugly.

On the way, I removed unused "GlobFinder", which was unused and was not able to find versions in "migrations-directory"-subdirectories, as RecursiveRegexFinder does.

@mikeSimonson
Copy link
Contributor

@SchulteMarkus Thanks for your PR but, you are trying to do way to many things at the same time and it become really hard to review your change. There are a lot of indentation and documentation fix all over the place which is great but should be in their own PR.

I am not removing the GlobFinder before the 1.0 of doctrine migrations (hopefully in a week or so).

You don't explain what your code is supposed to do which also makes it harder for me to review.

Can you please provide more information ?

Thanks

@SchulteMarkus
Copy link
Author

@mikeSimonson, I am reducing the PR to the new configuration-option "organize-versions", on Wednesday or Thursday, hopefully. I will provide the additional code-changes (such as indentation, adding .editorconfig ...) in other PRs by time.
This PR needs GlobFinder to be removed, as described. If your are removing GlobFinder in the near future, PR could be merged afterwards (if it is fine for you on the other issues)?!

@mikeSimonson
Copy link
Contributor

@SchulteMarkus Well I still don't know what your feature want to achieve and how I should be using it?

@SchulteMarkus
Copy link
Author

@mikeSimonson I have updated the PR-description a bit. Better understandable now?

@mikeSimonson
Copy link
Contributor

Okay, yes I get it now.
You should still leave the GlobFilter as it is there for compatibility reasons and for people that thus won't use this option.
And then maybe add a check (if possible) that if the organize-versions option is set and the GlobFilter too, then output an error.

@SchulteMarkus
Copy link
Author

Closing here.
I will readd this PR later, from a "clean" branch (without indentation fixes etc) and with a clean branch-history.

@mikeSimonson
Copy link
Contributor

ok.
Don't forget about it. I think that it would be really useful.

@SchulteMarkus
Copy link
Author

See #310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants