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

Pull request automation: use full npm install #66314

Merged
merged 3 commits into from
Oct 22, 2024

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Oct 22, 2024

What?

Use full npm install for the pull-request-automation workflow.

This installation is larger and slower, however I suspect that is likely to be offset by caching.
This is also blocking work on the migration to npm workspaces that has its own merits in #66272.

Why?

The pull-request-automation workflow creates problems when migrating to npm workspaces:

https://github.com/WordPress/gutenberg/actions/runs/11435991643/job/31813748278

Extracted from #66272.

How?

Use the regular npm installation flows shared by other github actions in the project.

Testing Instructions

CI passes. pull-request-automation workflow runs as expected.

This is likely to be cached and creates fewer problems with installing a
specific workspace and patch package.
@sirreal sirreal added the [Type] Project Management Meta-issues related to project management of Gutenberg label Oct 22, 2024
@sirreal
Copy link
Member Author

sirreal commented Oct 22, 2024

The job on this branch reuses a cache and completes in a reasonable amount of time. It just took 37s. It doesn't appear significantly slower than other recent runs. They range from 17s to 32s.

@sirreal sirreal requested a review from gziolo October 22, 2024 10:32
@sirreal sirreal marked this pull request as ready for review October 22, 2024 10:32
@sirreal sirreal requested a review from desrosj as a code owner October 22, 2024 10:32
Copy link

github-actions bot commented Oct 22, 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: sirreal <[email protected]>
Co-authored-by: gziolo <[email protected]>

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

@sirreal sirreal requested a review from t-hamano October 22, 2024 10:52
@gziolo
Copy link
Member

gziolo commented Oct 22, 2024

Screenshot 2024-10-22 at 12 58 17

I don't think to make sense anymore to have a different installation step for this job given that the cache reuse is so fast.

@sirreal sirreal merged commit e43a1c5 into trunk Oct 22, 2024
64 checks passed
@sirreal sirreal deleted the pull-request-automation/use-regular-npm-install branch October 22, 2024 11:17
@github-actions github-actions bot added this to the Gutenberg 19.6 milestone Oct 22, 2024
@getdave getdave added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 25, 2024
@getdave
Copy link
Contributor

getdave commented Oct 25, 2024

Backporting this on advice from @sirreal in relation to failing CI on the wp/6.7 branch

cc @kevin940726 @ellatrix @cbravobernal

Copy link

There was a conflict while trying to cherry-pick the commit to the wp/6.7 branch. Please resolve the conflict manually and create a PR to the wp/6.7 branch.

PRs to wp/6.7 are similar to PRs to trunk, but you should base your PR on the wp/6.7 branch instead of trunk.

# Checkout the wp/6.7 branch instead of trunk.
git checkout wp/6.7
# Create a new branch for your PR.
git checkout -b my-branch
# Cherry-pick the commit.
git cherry-pick e43a1c574959eb835f34b92479b7b6b4ff5ff8ea
# Check which files have conflicts.
git status
# Resolve the conflict...
# Add the resolved files to the staging area.
git status
git add .
git cherry-pick --continue
# Push the branch to the repository
git push origin my-branch
# Create a PR and set the base to the wp/6.7 branch.
# See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request.

@getdave
Copy link
Contributor

getdave commented Oct 25, 2024

Ok then...I'll do this by hand.

getdave pushed a commit that referenced this pull request Oct 25, 2024
Remove single-package installation from this workflow that creates problems
when migrating to npm workspaces.

The regular npm install workflow is frequently available in cache.

---

Co-authored-by: sirreal <[email protected]>
Co-authored-by: gziolo <[email protected]>
# Conflicts:
#	.github/workflows/pull-request-automation.yml
getdave added a commit that referenced this pull request Oct 25, 2024
Remove single-package installation from this workflow that creates problems
when migrating to npm workspaces.

The regular npm install workflow is frequently available in cache.

---

Co-authored-by: sirreal <[email protected]>
Co-authored-by: gziolo <[email protected]>
Co-authored-by: Jon Surrell <[email protected]>
Co-authored-by: sirreal <[email protected]>
Co-authored-by: kevin940726 <[email protected]>
@getdave getdave added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Oct 25, 2024
@getdave
Copy link
Contributor

getdave commented Oct 25, 2024

Manually backported in #66447

karthick-murugan pushed a commit to karthick-murugan/gutenberg that referenced this pull request Nov 13, 2024
Remove single-package installation from this workflow that creates problems
when migrating to npm workspaces.

The regular npm install workflow is frequently available in cache.

---

Co-authored-by: sirreal <[email protected]>
Co-authored-by: gziolo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Type] Project Management Meta-issues related to project management of Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants