-
Notifications
You must be signed in to change notification settings - Fork 91
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: migrate from CircleCI to Github Actions #1836
Conversation
This reverts commit 96f6960.
.github/workflows/ci.yaml
Outdated
- name: Upload dist artifacts | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: dist-artifact | ||
path: packages/**/dist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stores content of packages/**/dist/
for test
and publish
jobs.
To (hopefully 😅) stay within the 500MB storage monthly allowance, we can explicitly set retention-days
in the config:
Ex:
- name: Upload dist artifacts | |
uses: actions/upload-artifact@v4 | |
with: | |
name: dist-artifact | |
path: packages/**/dist | |
- name: Upload dist artifacts | |
uses: actions/upload-artifact@v4 | |
with: | |
name: dist-artifact | |
path: packages/**/dist | |
retention-days: 7 |
Or globally define it from the settings page.
Which way do you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something we had set previously for CCI? I didn't see an equivalent, so I could go either way. The config route does feel more obvious for us to easily grok in the future, but the settings live independent of commit history.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's definitely rely on the setting rather than specify here. I want to be able to control this outside the scope of a PR.
1a09da3
to
083e01b
Compare
083e01b
to
d865963
Compare
.github/workflows/ci.yaml
Outdated
@@ -0,0 +1,159 @@ | |||
name: CI | |||
|
|||
on: push |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these CI workflows should only happen on pull_request
. That's the typical setting for all CircleCI repos. We just turned it off temporarily on react-components
because there was no way to get next
to build in addition to main
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this config/GHA now allow us to only build on PR for select branches? 🤞🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it supports it.
on:
pull_request:
branches:
- 'releases/**'
.github/workflows/ci.yaml
Outdated
- name: Upload dist artifacts | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: dist-artifact | ||
path: packages/**/dist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's definitely rely on the setting rather than specify here. I want to be able to control this outside the scope of a PR.
1ce6fa2
to
f566924
Compare
.github/workflows/ci.yaml
Outdated
NETLIFY_SITE_ID: ${{ secrets.NETLIFY_SITE_ID }} | ||
|
||
deploy-gh-pages: | ||
# if: contains(needs.initialize.outputs.teams, 'Writers') && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/next') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, this will only trigger following a push
event.
For push
events, github.ref
looks something like this refs/heads/{branch}
For pull_request
events, github.ref
> refs/pull/{PR#}/merge
bf2975f
to
abf1547
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @ze-flo. I'll plan to continue on with this PR in prep for final transition away from CircleCI
publish: | ||
needs: [initialize, lint, test] | ||
runs-on: ubuntu-latest | ||
if: contains(needs.initialize.outputs.teams, 'Maintainers') && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/next') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition is fine for now, but we'll need to allow version maintenance branches (i.e. v7
, v8
, etc) as well, so may need to weigh the idea of removing and relying solely on Lerna's allowBranch
config.
Description
Migrate from CircleCI to Github Actions! 🤖 🚀
Detail
Checklist
[ ] 👌 design updates will be Garden Designer approved (add the designer as a reviewer)[ ] 🌐 demo is up-to-date (npm start
)[ ] ⬅️ renders as expected with reversed (RTL) direction[ ] 🤘 renders as expected with Bedrock CSS (?bedrock
)[ ] 💂♂️ includes new unit tests. Maintain existing coverage (always >= 96%)[ ] ♿ tested for WCAG 2.1 AA accessibility compliance[ ] 📝 tested in Chrome, Firefox, Safari, and Edge