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

ci: Update actions in GitHub workflows and cleanup #3093

Merged
merged 7 commits into from Apr 10, 2024

Conversation

justlevine
Copy link
Collaborator

@justlevine justlevine commented Apr 7, 2024

What does this implement/fix? Explain your changes.

This PR:

  • Updates the various GitHub actions used by our workflows to their latest versions.
  • Removes matrix testing from our workflows when only one value (e.g. php-version, node-version) used.
  • Replaces the use of styfle/cancel-workflow-action with the built-in concurrency property.
  • Bumps the default PHP version used for workflows to PHP 8.2
  • Updates the WordPress/PHP matrixes to make them more readable/manageable.

This PR does not

  • Bump the active Node version used for workflows to Node 20.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

Any other comments?

Once this is merged, we'll need to update the repo settings for required actions.

Where has this been tested?

Operating System: n/a (See action history for this PR)

WordPress Version: 6.5.0

@coveralls
Copy link

coveralls commented Apr 7, 2024

Coverage Status

coverage: 84.373%. remained the same
when pulling f57709f on ci/update-workflow-actions
into 4dae631 on develop.

@justlevine justlevine force-pushed the ci/update-workflow-actions branch 3 times, most recently from f6cbc20 to 6e5de4b Compare April 7, 2024 10:34
@justlevine justlevine added Status: In Review Needs to be reviewed by a project maintainer before merge scope: build scripts Affects build scripts, CI workflows, and automation. Type: Chore (updating CI tasks etc; no production code change) labels Apr 7, 2024
@justlevine justlevine force-pushed the ci/update-workflow-actions branch 3 times, most recently from 34368c2 to 33c6e97 Compare April 7, 2024 13:02
@justlevine
Copy link
Collaborator Author

justlevine commented Apr 7, 2024

WP 5.2 on PHP 7.3 is failing for NodeByUriTest:testDefaultTaxTermByUri, but I am unsure why get_term_by() doesn't return the valid default term 🤔

@jasonbahl @josephfusco any ideas (branch edits open, so feel free to fix directly on this branch if you have a solve)

@jasonbahl
Copy link
Collaborator

@justlevine I'm not sure we need to prioritize testing 5.2.

We haven't been testing against it before and according to the telemetry data we collect, we have 0 users on 5.2.

We have:

  • 25 users on WordPress 4.9
  • 0 users on WordPress 5.0
  • 0 users on WordPress 5.1
  • 0 users on WordPress 5.2
  • 38 users on WordPress 5.3
  • 47 user on WordPress 5.4
  • 55 users on WordPress 5.5
  • 52 users on WordPress 5.6
  • 131 users on WordPress 5.7
  • 408 users on WordPress 5.8
  • 3050 users on WordPress 5.9
  • And then more than that on each of the following: 6.0, 6.1, 6.2, 6.3, 6.4, 6.5

@justlevine
Copy link
Collaborator Author

justlevine commented Apr 9, 2024

@justlevine I'm not sure we need to prioritize testing 5.2.

We haven't been testing against it before and according to the telemetry data we collect, we have 0 users on 5.2.

It's less about 5.2 specifically, and more about testing our "minimum supported version" now and in the future.
5.2 is just the lowest version whose docker still builds. It gives us a nice spread as we work to 2.0, and lets us drop middle versions in the future without worrying too much about coverage.

If it's not worth fixing the Codeception test, I can keep bumping until we find a minimum version that just works.

@jasonbahl
Copy link
Collaborator

@justlevine ya, I think the effort is better spent ensuring things work on more recent versions and coming up with a plan / policy for supported versions that we can put into place and follow over time.

i.e. "we will officially support the latest xx versions of WordPress and the last xx versions of PHP" or something to that tune.

Definitely needs more thought than that, but I think there's more value in encouraging folks to update vs putting effort into testing versions we wouldn't actively recommend be used in the first place 🤷🏻‍♂️

@justlevine
Copy link
Collaborator Author

Definitely needs more thought than that, but I think there's more value in encouraging folks to update vs putting effort into testing versions we wouldn't actively recommend be used in the first place 🤷🏻‍♂️

💯

The goal of the matrix changes in this PR is not philosophical, only a cleaner restructure of the matrix to make it easier to add/change versions without losing coverage.

Will find a docker build that works as-is and ping you for re-review.

Copy link

codeclimate bot commented Apr 9, 2024

Code Climate has analyzed commit f57709f and detected 0 issues on this pull request.

View more on Code Climate.

@justlevine
Copy link
Collaborator Author

@jasonbahl WP 5.5 is the magic number.
Ready for your review.

@justlevine justlevine requested review from jasonbahl and removed request for jasonbahl April 9, 2024 22:20
@jasonbahl jasonbahl merged commit ce49102 into develop Apr 10, 2024
31 checks passed
@jasonbahl jasonbahl mentioned this pull request Apr 10, 2024
@justlevine justlevine deleted the ci/update-workflow-actions branch April 10, 2024 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: build scripts Affects build scripts, CI workflows, and automation. Status: In Review Needs to be reviewed by a project maintainer before merge Type: Chore (updating CI tasks etc; no production code change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants