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

Merge 18.4.0.2 into develop #17318

Closed
wants to merge 9 commits into from
Closed

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Oct 14, 2021

Includes:

Conflicts on Podfile and Podfile.lock, I solved them by keeping 1.63.1, the version on the release branch. On develop, we a 1.64.0 pre-release but it doesn't yet contain those fixes. They'll come in when a new pre-release or a stable version of 1.64.0 is cut, as they are already in Gutenberg mobile's develop: wordpress-mobile/gutenberg-mobile#4113.

Nothing to test, if CI Shows green, we're good.

Regression Notes

  1. Potential unintended areas of impact
    N.A.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N.A.

  3. What automated tests I added (or what prevented me from doing so)
    N.A.


  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes. N.A.
  • I have considered adding accessibility improvements for my changes. N.A.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

dcalhoun and others added 9 commits October 12, 2021 16:48
…elease_1.63.1

Integrate gutenberg-mobile release 1.63.1
….0.2-into-develop

Conflicts on `Podfile` and `Podfile.lock`, I solved them by keeping
1.63.1, the version on the release branch. On `develop`, we a 1.64.0
pre-release but it doesn't yet contain those fixes. They'll come in when
a new pre-release or a stable version of 1.64.0 is cut, as they are
already in Gutenberg mobile's develop:
wordpress-mobile/gutenberg-mobile#4113
@peril-wordpress-mobile
Copy link

You can trigger an installable build for these changes by visiting CircleCI here.

@mokagio mokagio requested a review from a team October 14, 2021 09:57
@mokagio mokagio added this to the 18.4 ❄️ milestone Oct 14, 2021
@mokagio mokagio changed the title Release script: Update gutenberg-mobile ref Merge 18.4.0.2 into develop Oct 14, 2021
@peril-wordpress-mobile
Copy link

Messages
📖 This PR has the 'Releases' label: some checks will be skipped.

Generated by 🚫 dangerJS

@AliSoftware
Copy link
Contributor

AliSoftware commented Oct 14, 2021

Conflicts on Podfile and Podfile.lock, I solved them by keeping 1.63.1, the version on the release branch. On develop, we a 1.64.0 pre-release but it doesn't yet contain those fixes.

@mokagio I usually solve it the opposite way, keeping what's in develop to resolve the conflict. Because if you make 1.63.1 land in develop, any new feature worked on in develop which depends on what was introduced in 1.64.0-alpha* (a very likely scenario to exist, otherwise why would they have landed the alpha in develop already if no new feature needed it?) will break if you make develop revert to 1.63.1.

The team usually integrate a new version in develop very soon after fixing things in release — so that develop ultimately include the same fix — so imho better not break develop, and then wait for the fix to be duplicated there soon, rather than introducing a regression in develop via this PR landing.

Would love to hear @dcalhoun confirm (or deny) this though.

gutenberg :tag => 'v1.64.0-alpha2'
gutenberg :tag => 'v1.63.1'
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like the wrong conflict resolution to me (see previous comment)

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, the correct conflict resolution would be the newer tag within develop1.64.0-alpha2.

@dcalhoun
Copy link
Member

dcalhoun commented Oct 14, 2021

Because if you make 1.63.1 land in develop, any new feature worked on in develop which depends on what was introduced in 1.64.0-alpha* (a very likely scenario to exist, otherwise why would they have landed the alpha in develop already if no new feature needed it?) will break if you make develop revert to 1.63.1.

This is correct. The very existence of an alpha tag in the host WordPress app repos means something in Gutenberg is dependent upon the changes in the alpha release. Reverting from an alpha to a lower release will likely break the Gutenberg develop branch for local development against the WordPress host apps.

The team usually integrate a new version in develop very soon after fixing things in release — so that develop ultimately include the same fix — so imho better not break develop, and then wait for the fix to be duplicated there soon, rather than introducing a regression in develop via this PR landing.

This is somewhat true. For better or worse, we only land alpha releases into the WordPress host apps when Gutenberg depends upon the changes the release contains. For the fix in 1.63.1, it was a fix for something completely contained in Gutenberg. So, it is unlikely we will land it into the WordPress apps develop until (A) the next release (1.64.0) or (B) an alpha release for an unrelated feature (1.65.0-alpha.X).

If this practices causes confusion or trouble, we could discuss immediately landing any change, but we avoid doing so because it is currently a fair amount of manual work to land changes in both WordPress host apps. 😩

@mokagio
Copy link
Contributor Author

mokagio commented Oct 15, 2021

Lol and that's why I shouldn't open merge PR in the evening 🤦‍♂️

@mokagio mokagio closed this Oct 15, 2021
@mokagio mokagio deleted the merge/release-18.4.0.2-into-develop branch October 15, 2021 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants