Document changes in FileInfo and FilePart in Migration27.md

[octonato]

PR #8878 introduces a new dispositionType: String on FileInfo and FilePart, but this is not being mentioned on the migration guide.

play-scala-fileupload-example sample got broken because of that.
https://travis-ci.org/playframework/play-scala-fileupload-example/jobs/478091440#L951-L956

[dwijnand]

More work because of last-minute backporting of changes...

[mkurz]

Hmm.. We might already have migration notes for this... Have a look at the migration notes in my open pull request https://github.com/playframework/playframework/pull/8913/files

If you accept that pull request we can just updated the section there by also mentionig dispositionType (?)

[dwijnand]

I don't think we should introduce another breaking change with that PR.

[mkurz]

Hmm... But FilePart already is broken by requiring dispositionType when used in case statement.
Now #8913 requires to add fileSize as well... So for migration this is not really another breaking change, in total it's more or less just one (by requiring two params, this can be documented in one section).

[dwijnand]

Sounds like #8878 wasn't ready to be merged then.

[octonato]

@mkurz, although I agree that is one breaking change affecting two fields, we need to cut things somewhere.

It was a mistake to introduce a breaking change in an RC. We need to stabilize it otherwise we never get to release 2.7.0.

May I ask you to split the changes on the migration guide in #8913 and send it in a separated PR?

#8913 will need to be scheduled for 2.8.x.

[mkurz]

@dwijnand @renatocaval I agree that it was a mistake to merge #8878 in the final RC phase. I shouldn't have labeled it needs-backport... Sorry for that. @marcospereira probably also overlooked that it was breaking when he backported it (A pity that SI-6524 isn't resolved yet...)

I also 100% fully agree it's finally time to release 2.7.0 soon - and I am looking forward to it.

However please consider this thought:
The mistake was made already, a migration step needs to be done now (even though this was not my intention, sorry again).
So now #8913 again introduces a breaking change to FilePart (when used in case), exactly like #8878 did. Given that most people will upgrade from 2.6.x to 2.7.x (and not from RC to RC) the majority of play users would see those two breaking changes (of adding two fields to FilePart) as one migration step only - even though it would be introduced by two pull requests (if #8913 makes it into 2.7.x) (and by two RC's).

Even if it sounds odd, but the mistake of introducing this breaking change in RC9 would give us the opportunity/chance now to again introduce another breaking change in the next RC (or final, whatever it is) - that, again, eventually will just look like one single breaking change (adding two fields instead of just one to FilePart when used in case).
Please also consider that this breaking change does not effect all users, but only those that use a custom body parser for multipart file uploads. I guess that will be a minority.
Another thing to consider: In case we merge #8913 for 2.8.x someday (but not backport it to 2.7.x) there would be another migration step necessary from 2.7.x to 2.8.x. However given that people already have to do exactly this migration step from 2.6.x to 2.7.x, wouldn't it make sense to avoid a breaking change for 2.8.x again, by just breaking it once (by adding both fields in 2.7.0)?

@marcospereira What's your oppinion on this? Should we also merge #8913 (after reviewing of course) given that the mistake was made already?

(Sorry if too long, it's late, I am tired)

[dwijnand]

I'd be fine with backporting straight into 2.7.0. I just don't want an RC10.

[mkurz]

I will update the migration notes in #8913 tomorrow.

[marcospereira]

Hey @mkurz, just to let you know that we agree to backport this and, since it is not a significant break, we won't need another RC.

[mkurz]

Alright, I will update the migration notes in #8913 then.

[mkurz]

I updated the migration notes in #8913 to include the changes in FileInfo and FilePart.