-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
[stable29] Migration Attributes #46889
Conversation
This PR adds a new API. Are you sure this will be backported? |
Discussion/Decision has been to have it backported to v29, so people can "experience" the new feature when updating to v30 - given that we judge this change to be low/no risk. Thus adding @sorbaugh to clarify/ensure this. |
As always, features = minor. |
Maybe we can wait until it was tested a couple of times during the 30 beta/rc updates? |
During the betas sure, maybe RC1 / RC2 max, else there isn't enough time to actually get feedback for it. |
well the updates within the 30 beta X to beta X+1 also use this right? not only the updates from 29 to 30 |
Yes. My point is rather when we consider it "the last point in time we are (relatively) fine with merging it", hence I do not want to wait until the very last minute before that v29 package is build and released but also respect v29 release timelines. |
20cd7aa
to
e027825
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Psalm found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
e027825
to
7b67067
Compare
a329ce1
to
f3ac8ad
Compare
|
@ArtificialOwl beyond testing if the occ command works, did (and if not can) you test that the 29+PR upgraded to 30-master (with above annotated migrations) still runs through smoothly? That would be a pre-requisite in terms of test-set that should be checked before merging this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good with one question.
Also please update the issue description to reflect the current state of the partial backport. Currently it looks like there is still work to do but the diff seems fine
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
f3ac8ad
to
825be0a
Compare
I have updated the description of the PR |
i will test 29.0.4+PR with an upgrade to 30.0.0beta5 after its release at the end of this week. (But I see no reason for a fail) |
Thanks a lot @ArtificialOwl 👍👍👍 |
Testing has been reported as being successful upgrading 29+this-pr to v30-pre-releases |
Backport of #46476+#47069
This is a light backport of the Migration Attributes + Metadata Manager adding the features to:
description()
method to obtain human readable content,migrations:preview
occ command to display future migrationsThe backport removes the part of creating metadata based on local content, as it is useless for NC29, and not compat for php 8.0