Add warning for diffCommand

[VincentLanglet]

Q	A
Type	improvement
BC Break	yes
Fixed issues	#930

Summary

Add warning when running diff command with executed unavalaible or non executed migrations.

[goetas]

I think that you should be targeting the master branch

[goetas]

That seems to be the reason why tests are failing

[VincentLanglet]

@goetas I fix some stuffs, but I don't understand the static analysis failure.

[VincentLanglet]

@goetas @alcaeus Phpstan is fixed now. Is the Scrutinizer check required ?

[SenseException]

I've restarted the check and it's green now.

[goetas]

I'm not sure if we need that many warnings... I was just thinking about adding a a warning but keep the flow exactly as before. I'm this pr in the worst case we as to the user confirmation for two times...

Opinions?

[VincentLanglet]

I'm not sure if we need that many warnings... I was just thinking about adding a a warning but keep the flow exactly as before. I'm this pr in the worst case we as to the user confirmation for two times...

IMO, you barely never want to create a new migration with non executed ones or executed non available one because it will generate some conflict between the migrations.

The goal of adding these warning was to prevent the developper from messing up BEFORE the migrations was created.

If you look at the issue doctrine/DoctrineMigrationsBundle#177, you'll have opinion. They wanted to not allow running diff command when not in sync, or to run migrate before diff. This is going in my way.

[goetas]

I'm convinced then. Looks good to me.

Can you add tests please for this feature?

[VincentLanglet]

@goetas Tests added

[SenseException]

I'd like to see the true case of these two new methods covered too. You can have an early exit after their if-blocks are done by having the empty diff functionality.

[VincentLanglet]

@SenseException Sorry, I don't understand what you mean by the true case ?
There was already a test for this command. I added the case with non-executed migration or executed-non-available one.

[goetas]

Please see #948

[VincentLanglet]

@goetas Should I close my PR in favor of yours ?
It seems indeed better with only one prompt.

[goetas]

yeah, I suggest to move the discussion to #948