-
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
Conversation
whitelisab
left a comment
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.
Just one question about which vault-secrets file to keep!
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 think we need to keep this file vault-secrets.yaml and remove vault-secrets.yml instead
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.
Was the package-lock.json just outta sync?
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 it was, the job was failing with npm ci since it had a stale package-lock
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 main workflow, then essentially we're only allowing us to trigger releases.
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Restore the build folders |
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.
question: Do we need to also build before caching?
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 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
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.
|
Oops had auto-merge enabled on this so it was merged before talking through the questions here. Sorry about that! |
Summary
Updates GHA workflow and removes use of circle ci.
Description
The new workflow allows us to more easily add additional jobs that we want to run against our main branch. Currently starts simple with just building and linting the project on PRs and pushes.
Ticket: https://contentful.atlassian.net/browse/DX-242