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

Extended details on up to date migrations #917

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

goetas
Copy link
Member

@goetas goetas commented Jan 15, 2020

Q A
Type improvement
BC Break -
Fixed issues #886

Summary

alternative version to #886

@goetas
Copy link
Member Author

goetas commented Jan 15, 2020

@dbu can you have a look at this?

Copy link
Member

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great!

@goetas
Copy link
Member Author

goetas commented Jan 16, 2020

i'm exploring the idea to use the -vv option (verbosity level) to instead of --extended to decide if print or not the extra table... what do you think?

(if accepted, this could be applied to all the other commands...)

@dbu
Copy link
Member

dbu commented Jan 16, 2020

if using verbosity, i would use single -v, otherwise there might be too much noise from symfony debugging things. it sounds kind of natural to do that, but then again i don't know if there are other things reacting to the added verbosity. and having an explicitly named parameter would make it easier to spot the feature, though the description of the command could mention -v. another option could be to print by default but not with -q.

really not sure which is better, but don't think its terribly important either. maybe check if -v (or -vv) prints much extra stuff in a vanilla symfony installation, and if not, go with that?

@alcaeus
Copy link
Member

alcaeus commented Jan 16, 2020

If I understand it correctly, the current extended option shows a detailed table of all migrations instead of just saying "up-to-date" or "out-of-date"? If so, I would avoid using the verbosity switches (either -q or -v) for this, as those imply that they print more or less debug output. Maybe --show-migration-info or --list-migrations or something similar can be used to better convey what the setting does. --extended doesn't quite say what's going to change if I use the option.

@dbu
Copy link
Member

dbu commented Jan 16, 2020

i think i agree with @alcaeus point. also about using a more specific name than some "synonym" for verbosity. --show-invalid-migrations maybe?

@goetas goetas force-pushed the up-to-date-command-extended branch from 4b339ae to fcc4567 Compare January 16, 2020 08:01
@goetas goetas force-pushed the up-to-date-command-extended branch from fcc4567 to 849712c Compare January 16, 2020 08:06
@goetas
Copy link
Member Author

goetas commented Jan 16, 2020

@alcaeus @dbu thank for the feedback! i have implemented it now using --list-migrations since the output can contain both unexecuted and missing migrations.

@goetas goetas marked this pull request as ready for review January 16, 2020 08:09
@goetas goetas added this to the 3.0.0 milestone Jan 16, 2020
@goetas goetas merged commit 93927c3 into doctrine:master Jan 16, 2020
@goetas goetas deleted the up-to-date-command-extended branch January 16, 2020 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants