CI - Make workflows sequential + concurrency mode + lint lockfile #163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MrChocolatine
commented
Oct 27, 2021
|
|
||
| jobs: | ||
| create-git-tag: | ||
| if: ${{ github.event.workflow_run.conclusion == 'success' }} |
There was a problem hiding this comment.
This if condition is needed because:
https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows#workflow_run
workflow_run
(...)
A workflow run is triggered regardless of the result of the previous workflow.
MrChocolatine
commented
Oct 27, 2021
| branches: | ||
| - master | ||
| workflow_run: | ||
| workflows: CI |
There was a problem hiding this comment.
The name of the workflow we want to wait for:
https://github.com/peopledoc/ember-cli-embedded/blob/v2.3.1/.github/workflows/ci.yml#L1
MrChocolatine
changed the title
tag-release-publish only after lint & tests passed
MrChocolatine
force-pushed
the
ci-only-deploy-if-tests-passed
branch
5 times, most recently
from
48c4a66 to
acbeff9
Compare
MrChocolatine
commented
Oct 28, 2021
Comment on lines
+12
to
+14
| concurrency: | ||
| group: ${{ github.head_ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Currently the workflows `CI` and `tag-release-publish` are executed in parallel: - Workflow `CI` https://github.com/peopledoc/ember-cli-embedded/runs/4020380098?check_suite_focus=true#step:1:1 - Workflow `tag-release-publish` https://github.com/peopledoc/ember-cli-embedded/runs/4020380101?check_suite_focus=true#step:1:1 By enabling the option `Show timestamps`, using the cog on the right, we can see both started at the same time (+/- a few seconds of difference). Which is wrong as we could potentially tag/release/publish while the lint/tests failed. (this is unlikely to happen since the workflow `CI` already runs for any pull request, but better safe than sorry right?) --- This `if` condition is needed because: https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows#workflow_run > `workflow_run` > (...) > A workflow run is triggered regardless of the result of the previous workflow.
MrChocolatine
force-pushed
the
ci-only-deploy-if-tests-passed
branch
from
acbeff9 to
f766b67
Compare
MrChocolatine
changed the title
tag-release-publish only after lint & tests passed
MrChocolatine
force-pushed
the
ci-only-deploy-if-tests-passed
branch
from
43a422e to
6ebd9a5
Compare
As of now, `npx` in npm v6 does not prompt about the package about to be used. But things change with npm v7 where `npx` now prompts before installing anything. https://docs.npmjs.com/cli/v7/commands/npx > To prevent security and user-experience problems from mistyping > package names, `npx` prompts before installing anything. Suppress this > prompt with the `-y` or `--yes` option. However this flag `--yes` is not supported by npm v6 and would result in an error if used. We need to set that flag through `npm_config_`: https://docs.npmjs.com/cli/v6/using-npm/config > Any environment variables that start with `npm_config_` will be > interpreted as a configuration parameter. For example, putting > `npm_config_foo=bar` in your environment will set the `foo` > configuration parameter to `bar`. Any environment configurations that > are not given a value will be given the value of true. Config values > are case-insensitive, so `NPM_CONFIG_FOO=bar` will work the same. See: - https://dev.to/omrilotan/npx-breaking-on-ci-48ak
MrChocolatine
force-pushed
the
ci-only-deploy-if-tests-passed
branch
from
6ebd9a5 to
000dd9e
Compare
ndekeister-us
approved these changes
Oct 29, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CI
Run workflow
tag-release-publishonly after lint & tests passed (#163)Make the workflow
tag-release-publishwaits for the workflowCIto finish, so that they run sequentially (the same way jobs' steps are run sequentially).Ensure only 1 workflow runs at a time (#163)
Surprisingly, while a new run of the workflow is triggered when pushing new changes, any previous workflow still running is not cancelled, resulting in a waste of resources.
We now use the
concurrencysetting to ensure that only a single workflow, using the same concurrency group, will run at a time.Lint
yarn.lockfile to spot malicious attacks (#163)