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

Cherry pick automation: fix for forks #62900

Merged
merged 5 commits into from
Jul 4, 2024
Merged

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jun 27, 2024

What?

This workflow was introduced in #62716, and it generally works well. The problem is that when a PR is merged from a fork, the workflow runs in the context of that fork and lacks the token.

The idea of this PR is to move automation from PRs to pushes. I'll first test this on a test/trunk branch.

Why?

Because it doesn't currently work for PRs from forks, such as #62857.

How?

Testing Instructions

It works:

Labelling after a merge still works too, but worth noting that labelling after will still not work on forks because this workflow must run on the PR:

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 Jun 27, 2024
@ellatrix ellatrix requested a review from desrosj as a code owner June 27, 2024 05:17
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
gutenbergplugin pushed a commit that referenced this pull request Jun 27, 2024
@ellatrix ellatrix force-pushed the fix/cherry-pick-automation-forks branch from 74d73a1 to 66e9903 Compare June 27, 2024 05:58
Copy link

github-actions bot commented Jun 27, 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 <ellatrix@git.wordpress.org>
Co-authored-by: vcanales <vcanales@git.wordpress.org>

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

ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
ellatrix added a commit that referenced this pull request Jun 27, 2024
gutenbergplugin pushed a commit that referenced this pull request Jun 27, 2024
Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

Labelling after a merge still works too, but worth noting that labelling after will still not work on forks because this workflow must run on the PR

If this means that we'd have to cherry-pick those manually I think it's fine, but could we add some indication that some PRs are "missing"?

My concern is that the presence of automation in part of the process might relax those in charge of the release if we don't state explicitly that they still have to check the cherry picks.

@ellatrix
Copy link
Member Author

If this means that we'd have to cherry-pick those manually I think it's fine, but could we add some indication that some PRs are "missing"?

We anyway still have to check the label (which would be kept). There might be other job runs that failed for some other reason. It happens. At some point I'll update the handbook and I will mention that it's still required to check the label manually.

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

LGTM :)

@ellatrix ellatrix merged commit f90a132 into trunk Jul 4, 2024
62 checks passed
@ellatrix ellatrix deleted the fix/cherry-pick-automation-forks branch July 4, 2024 15:26
@github-actions github-actions bot added this to the Gutenberg 18.8 milestone Jul 4, 2024
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jul 18, 2024
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: vcanales <vcanales@git.wordpress.org>
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.

2 participants