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

chore(ci): refactor CI and upgrade actions #10294

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Conversation

jonkoops
Copy link
Contributor

@jonkoops jonkoops commented Apr 22, 2024

Splits the PR preview action into two distinct actions, one to build and deploy the documentation for the PR, and another to do the CI tasks such as linting, testing and building. The latter workflow no longer requires the pull_request_target to run, which limits access to repository secrets.

These workflows are now use a composite action to take care of the repetitive task of setting up Node.js, installing dependencies and running the build. The release workflow has been converted to re-use the CI and documentation workflows to reduce duplication.

I've ran the CI on this on my fork for testing, which you can see the result of here:

Push on main
CI — https://github.com/jonkoops/patternfly-react/actions/runs/8819259805
Release (includes docs) — https://github.com/jonkoops/patternfly-react/actions/runs/8819259882

Pull request
CI — https://github.com/jonkoops/patternfly-react/actions/runs/8819461879
Documentation — https://github.com/jonkoops/patternfly-react/actions/runs/8819461789/job/24210724184

@jonkoops jonkoops marked this pull request as draft April 22, 2024 12:12
@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 22, 2024

@jonkoops jonkoops marked this pull request as ready for review April 22, 2024 12:45
@jonkoops jonkoops marked this pull request as draft April 22, 2024 20:05
@jonkoops jonkoops changed the title chore(ci): split PR preview action and upgrade deps chore(ci): refactor CI and upgrade actions Apr 22, 2024
@jonkoops jonkoops force-pushed the upgrade-ci branch 2 times, most recently from d090156 to d006998 Compare April 23, 2024 11:26
@@ -77,9 +77,9 @@
"clean:build": "rimraf .cache .eslintcache coverage",
"clean:exports": "lerna run clean:exports --parallel --stream",
"generate": "yarn plop",
"lint": "node --max-old-space-size=4096 node_modules/.bin/eslint --ext js,jsx,ts,tsx --cache",
"lint": "node --max-old-space-size=4096 node_modules/.bin/eslint --ext js,jsx,ts,tsx --cache --cache-strategy content",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since Git doesn't track timestamps on files for modifications, the linter cache would not work. I've set this to check the file contents instead, which is slower, but will actually hit the cache.

@jonkoops jonkoops marked this pull request as ready for review April 24, 2024 15:56
Comment on lines -11 to -14
const owner = process.env.CIRCLE_PROJECT_USERNAME || ghrepo.split('/')[0]; // patternfly
const repo = process.env.CIRCLE_PROJECT_REPONAME || ghrepo.split('/')[1];
const prnum = process.env.CIRCLE_PR_NUMBER || process.env.GH_PR_NUM;
const prbranch = process.env.CIRCLE_BRANCH || process.env.GITHUB_REF.split('/').pop();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Circle CI isn't used anymore, so I've removed references to environment variables related to it.

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

This is incredible 🔥 🔥 🔥

Copy link
Contributor

@dlabaj dlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@dlabaj dlabaj merged commit 07e0116 into patternfly:main Apr 30, 2024
22 checks passed
@jonkoops
Copy link
Contributor Author

Thanks y'all! 🙌

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.

4 participants