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

Workflows: try a backport changelog #61785

Merged
merged 23 commits into from
May 20, 2024
Merged

Workflows: try a backport changelog #61785

merged 23 commits into from
May 20, 2024

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented May 20, 2024

What?

This PR introduces a test that checks if an entry with a core backport PR has been added to the backport changelog.

Entries are added as files to prevent merge/rebase conflicts. The expectation would be one file per core backport PR, which can contain a list of multiple Gutenberg PRs.

The files can be sorted per release: 6.6 changes go in the 6.6 folder.

Why?

The goal of this PR is to make sure that core backport PRs are created before a PR that requires a backport is merged into GB. This encourages PR authors to think about how it will be absorbed into core early, increases visibility, and allows us to merge the core PRs as soon as possible, which prevents a "queue" of many PRs to be backported (that may depend on one another).

The PR author will have to create a backport at some point anyway. I think it's more productive to do it sooner while it’s fresh and while you’re looking at the core files that you’re filtering/overriding.

If there's an existing (and open) core backport PR that a change is related to, it's ok to use that existing core PR. So if you wish to make multiple successive PHP changes, it's fine to start a backport PR and use that for all these related PRs. This makes it easier for others to know which PRs belong together (normally when a backport issue is created, it's really hard to figure out which PRs belong together and can go into the same backport PR).

Also note that you don't need to open a backport PR as soon as you create a GB PR, it should just be done before merging to trunk. Maybe we can skip running this test for draft PRs.

Probably we'll go through some time where we have remaining backports (#61226) and it's not possible to create a backport PR without previous changes. We are now two weeks from Beta 1, so we should prioritise getting those remaining backports merged as soon as possible.

If this works well, I can see it replace the backport issue entirely, or it becomes just of formality where we double check all the changes, but almost no work is needed.

How?

This script is inspired by the components changelog: we check a special changelog whenever a PHP change is made.

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

@ellatrix ellatrix added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label May 20, 2024
@ellatrix ellatrix requested a review from desrosj as a code owner May 20, 2024 06:37
Copy link

github-actions bot commented May 20, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ellatrix <[email protected]>
Co-authored-by: priethor <[email protected]>
Co-authored-by: vcanales <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

github-actions bot commented May 20, 2024

This pull request changed or added PHP files in previous commits, but none have been detected in the latest commit.

Thank you! ❤️

@vcanales
Copy link
Member

...make sure that core backport PRs are created as soon as a change is merged in GB that requires a backport.

I understand this as the PR has to be created and added to the changelog before the change is merged into PR, no? Apologies if this sounds pedantic, I'm just a bit confused by the order of operations :)

Maybe this PR obsoletes the other PHP change detection test.

Perhaps a combination rather than a replacement? I think it makes sense to have only one comment addressing PHP changes that need to be Backported and added to the changelog.

@ellatrix
Copy link
Member Author

Changed the wording

Perhaps a combination rather than a replacement? I think it makes sense to have only one comment addressing PHP changes that need to be Backported and added to the changelog.

Yes, we can keep both if it's useful

Copy link

github-actions bot commented May 20, 2024

Flaky tests detected in 306f737.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9162267686
📝 Reported issues:

Copy link
Contributor

@priethor priethor left a comment

Choose a reason for hiding this comment

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

It's a good, low-risk first attempt, and I see no reason not to try it and iterate later.

.github/workflows/check-backport-changelog.yml Outdated Show resolved Hide resolved
.github/workflows/check-backport-changelog.yml Outdated Show resolved Hide resolved
backport-changelog/readme.md Outdated Show resolved Hide resolved
@ellatrix ellatrix enabled auto-merge (squash) May 20, 2024 16:09
@ellatrix
Copy link
Member Author

Let's try this at least until the middle of the next release and see if there are benefits. We can add more ignored paths if necessary.

@ellatrix ellatrix disabled auto-merge May 20, 2024 16:59
@ellatrix ellatrix enabled auto-merge (squash) May 20, 2024 17:03
@ellatrix ellatrix disabled auto-merge May 20, 2024 17:13
@ellatrix ellatrix enabled auto-merge (squash) May 20, 2024 17:13
@ellatrix ellatrix merged commit ae6cf0c into trunk May 20, 2024
61 checks passed
@ellatrix ellatrix deleted the try/backport-pr-changelog branch May 20, 2024 17:57
@github-actions github-actions bot added this to the Gutenberg 18.5 milestone May 20, 2024
@ramonjd
Copy link
Member

ramonjd commented May 20, 2024

This PR introduces a test that checks if an entry with a core backport PR has been added to the backport changelog.

Hi there!

Thanks for this - going to help with release season for sure.

Quick question: is there a way to opt-out for PHP changes that don't require backports, e.g., PRs that sync from Core -> Gutenberg

Or this one I'm working on

@ramonjd
Copy link
Member

ramonjd commented May 20, 2024

Quick question: is there a way to opt-out for PHP changes that don't require backports, e.g., PRs that sync from Core -> Gutenberg

A "try" branch here skipping via a label:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants