Update GitHub CI workflow #19
Conversation
whymarrh
force-pushed
the
update-actions-workflow
branch
2 times, most recently
from
e33dcf9
to
47e7eaa
Compare
This change updates the GitHub Action workflow used for CI, adding names to the jobs as well as a terminal job that will simplify the process of adding required status checks to branches. Having a known terminal job that is consistent across all projects makes it easy to add a required status check for a branch—one that will not change as new intermediate jobs are added. A lint task and the appropriate `yarn` flags have been added as well.
whymarrh
force-pushed
the
update-actions-workflow
branch
from
47e7eaa
to
eed5dd9
Compare
|
I've created #20 to represent the work required to get the lint command going again |
| needs: | ||
| - build | ||
| steps: | ||
| - uses: actions/checkout@v2 |
There was a problem hiding this comment.
Could this step me omitted? 🤔 I don't see much utility in checking out the repo here.
There was a problem hiding this comment.
There needs to be some step and this seems like a harmless empty step
There was a problem hiding this comment.
OK, fair enough. I wasn't aware a step was required. We can replace this later with something more lightweight if need be.
whymarrh
force-pushed
the
update-actions-workflow
branch
2 times, most recently
from
a679f72
to
9771ad0
Compare
| on: | ||
| push: | ||
| branches: [ main ] | ||
| pull_request: |
There was a problem hiding this comment.
Note that this does need to be pull_request_target for any of our action workflows that require secrets, since those won't be provided on forks 🤔
pull_request seems perfectly fine to use on repos without secrets though - maybe even preferable, as testing changes to the workflows is easier with pull_request. So maybe this is fine as a default.
There was a problem hiding this comment.
Since we want to add npm publishing eventually, this seems like an important suggestion, though I'm not sure it needs to block this PR.
There was a problem hiding this comment.
Publishing would run post-merge, so that would be from the push event. I don't think using pull_request poses any challenges there.
My comment was more about status checks that use secrets, such as on metamask_extension where our CI builds have valid non-production Sentry and Matomo keys.
The "all-jobs-pass" job no longer checks out the repository. It had only included a checkout as a placeholder step [1], but this isn't necessary anymore now that we have the "Great success!" message. That can serve as our placeholder instead. [1]: #19 (comment)
The "all-jobs-pass" job no longer checks out the repository. It had only included a checkout as a placeholder step [1], but this isn't necessary anymore now that we have the "Great success!" message. That can serve as our placeholder instead. [1]: #19 (comment)
This change updates the GitHub Actions workflow used for CI, adding names to the jobs as well as a terminal job that will simplify the process of adding required status checks to branches.
Having a known terminal job that is consistent across all projects make it easy to add a required status check for a branch—one that will not change as new intermediate jobs are added.
I've added a lint step and the flags from our existing CircleCI config as well.