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

[CICD-606] Update actions/checkout@v4, node v20 #98

Merged
merged 2 commits into from
Sep 5, 2024

Conversation

mike-day
Copy link
Contributor

@mike-day mike-day commented Aug 29, 2024

JIRA Ticket

CICD-606

What Are We Doing Here

GHA currently uses an older version of both actions/checkout and Node. This PR updates to the newest versions we can use for both (considering transitive dependencies).

To install Node v20 using nvm, follow these steps:

  • Run nvm install v20
  • Start using v20 by running nvm use v20
  • To test your version, attempt running npm ci

Testing

Effectively testing compatibility for these dependency updates locally was tricky, but I was able to confirm several workflows ran checkout@v4 without errors.

Test e2e Deploy to WP Engine

act push -j run_action

Lint GHA Files

act push -j lint

The image used with act does not contain some packages available by default with GitHub, so both shellcheck and yamllint were installed as part of the workflow locally. The YAML for the job looked like this:

jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
      - name: Lint files
        uses: actions/checkout@v4
      - name: Update package list
        run: sudo apt-get update
      - name: Install ShellCheck
        run: sudo apt-get install -y shellcheck
      - name: Install yamllint
        run: sudo apt-get install -y yamllint
      - run: echo "Running shell script lint!"
      - run: find . -name "*.sh" -type f | xargs -I {} shellcheck --severity=error {}
      - run: echo "Running yml file lint!"
      - run: find . -name "*.yml" -type f | xargs -I {} yamllint {}

Version and Release

I couldn't figure out a great way to test this workflow locally. changesets/action expects a valid GitHub token to run, but supplying a real token could actually create a PR or release while testing, which is not ideal 😄

At one point, I did try simply commenting out the step to create a PR. checkout@v4 and setup-node@v2 both seemed to work as expected and the current version was output by the job.

Copy link

changeset-bot bot commented Aug 29, 2024

⚠️ No Changeset found

Latest commit: 91bf349

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mike-day mike-day force-pushed the cicd-606-checkout-v4 branch from 1e2b3cd to 502bec6 Compare September 3, 2024 17:17
@mike-day mike-day force-pushed the cicd-606-checkout-v4 branch from 502bec6 to 91bf349 Compare September 4, 2024 15:02
@mike-day mike-day marked this pull request as ready for review September 4, 2024 15:40
@mike-day mike-day requested a review from a team as a code owner September 4, 2024 15:40
Comment on lines +6 to +7
"node": ">=20",
"npm": ">=9"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@apmatthews I ended up going with the >= syntax.

While it could make sense to explicitly peg the version to 20.x here, we are already doing that with .nvmrc.

Also, it is a better dev experience to be more permissive with newer versions of NPM (it's easy enough to use any version of Node with NVM, but switching NPM on the fly is not really a thing). So it makes sense to match the expectation of allowing newer versions of both Node and NPM.

Lastly, it simply feels sensible to match the style/syntax in site-deploy.

@mike-day mike-day merged commit 00bd0b6 into main Sep 5, 2024
3 checks passed
@mike-day mike-day deleted the cicd-606-checkout-v4 branch September 5, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants