-
Notifications
You must be signed in to change notification settings - Fork 14
chore: update GHA workflow and remove circleci [DX-242] #11
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
Changes from all commits
f5008bf
a23fd1b
b45b331
78427bb
3525180
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to keep this file |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| name: Run Checks | ||
|
|
||
| on: | ||
| workflow_call: | ||
|
|
||
| jobs: | ||
| lint: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '22' | ||
| cache: 'npm' | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Restore the build folders | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Do we need to also build before caching?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that's taken care of in the build.yml file which runs before the check.yml and is a required pre-req in the main.yml
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| uses: actions/cache/restore@v4 | ||
| with: | ||
| path: build/ | ||
| key: build-cache-${{ github.run_id }}-${{ github.run_attempt }} | ||
|
|
||
| - name: Run Prettier | ||
| run: npx pretty-quick --check | ||
|
|
||
| - name: Run linting | ||
| run: npm run lint | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: ['*'] | ||
| pull_request: | ||
| branches: ['*'] | ||
|
|
||
| jobs: | ||
| build: | ||
| uses: ./.github/workflows/build.yml | ||
|
|
||
| check: | ||
| needs: build | ||
| uses: ./.github/workflows/check.yml |
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.
I was actually thinking about the whole github actions vs. circleCi discussion, and I 100% agree that tests/linting/building etc should be run in github actions so that external contributors can get insight into whether their builds are passing or not.
Releasing a new version feels like something that should be protected a little more, and in theory we should get that already with branch protections, but I wouldn't mind having an extra layer of security in that only contentful employees/automation bots that are authorized through circleCI can release code.
100% open to feedback and discussion, I'm still learning how all this works at Contentful, so I might be missing something.
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.
Yeah definitely worth having a discussion about. Another option to consider that could solve the same problem would be an additional branch protection that only allows certain contentful employees (The DX team) to actually merge code into main. If we then only had releases tied to our
mainworkflow, then essentially we're only allowing us to trigger releases.