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

handle unregistered migrations in up-to-date check #799

Merged
merged 1 commit into from Mar 21, 2019
Merged

handle unregistered migrations in up-to-date check #799

merged 1 commit into from Mar 21, 2019

Conversation

dbu
Copy link
Member

@dbu dbu commented Mar 20, 2019

Q A
Type bug & improvement
BC Break no
Fixed issues -

Summary

We need to handle the case where there are extra migrations in doctrine. without this change, the command outputs something like "Out-of-date! 18446744073709551613 migration is available to execute." we convert the signed integer value to unsigned.

with this fix, we explicitly handle the case. the default is to report the issue but not fail, same as the migrate command that only outputs a warning but does not refuse to run. i also add a flag in case people want a strict check.

));

return 1;
return true === $input->getOption('fail-on-unregistered') ? 2 : 0;
Copy link
Member Author

Choose a reason for hiding this comment

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

i originally had $input->getOption('fail-on-unregistered') ? 2 : 0 but then the code sniffer complained that the return value of getOption is mixed. i tried to fix it like this, but now the code style check complains about yoda style comparison. how should i do the comparison?

Copy link
Member

Choose a reason for hiding this comment

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

$input->getOption('fail-on-unregistered') === true

we need to handle the case where there are extra migrations in doctrine. without this change, the command outputs something like "Out-of-date! 18446744073709551613 migration is available to execute." the > 1 check says false and we convert the signed integer value to unsigned.
with this fix, we explicitly handle the case. the default is to report the issue but not fail, same as the migrate command that only outputs a warning but does not refuse to run. i also add a flag in case people want a strict check.
@jwage jwage added this to the 2.1.0 milestone Mar 20, 2019
@jwage jwage self-assigned this Mar 20, 2019
@dbu
Copy link
Member Author

dbu commented Mar 20, 2019

looks like scrutinizer failed talking to github or something.

is that ok as is, should i adjust anything with the behaviour?

@jwage
Copy link
Member

jwage commented Mar 20, 2019

Usually we just restart the build and try it again.

@dbu
Copy link
Member Author

dbu commented Mar 20, 2019

i don't think i have permission to do that.

content-wise, do you need anything more to have this mergeable? changelog entry maybe? and do you agree with the actual feature addition?

@jwage jwage merged commit 16a1cdf into doctrine:master Mar 21, 2019
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.

None yet

3 participants