ST-5122: Add GitHub Actions CI pipeline #14
Conversation
| @@ -8,7 +8,7 @@ | |||
| }, | |||
| "settings": { | |||
| "react": { | |||
| "version": "16.4" | |||
| "version": "17.2" | |||
There was a problem hiding this comment.
Unrelated, but as internally we only support 17 make this match. It is prompted by eslint-config-skyscanner, though React isn't actually used in this repo at all.
There was a problem hiding this comment.
I tried removing this locally to see what happens:
alakbarzeynalzade@M-LC2R24049H ~/dev/babel-plugin-transform-i18n add_gha ± npm run lint
> @skyscanner/[email protected] lint
> eslint *.js
Warning: React version was set to "detect" in eslint-plugin-react settings, but the "react" package is not installed. Assuming latest React version for linting.
There was a problem hiding this comment.
It doesn't break anything, but it is an annoying warning, so I can understand why some version was added as a default - may as well keep that up to date even though its currently non-functional :D
| @@ -1,12 +0,0 @@ | |||
| sudo: false | |||
There was a problem hiding this comment.
This file was not being used; delete.
| #!/bin/sh | ||
| . "$(dirname "$0")/_/husky.sh" | ||
|
|
||
| npm test |
There was a problem hiding this comment.
We're already doing a precommit on every commit; prepush is overkill; delete.
| @@ -1,4 +1,5 @@ | |||
| #!/bin/sh | |||
| . "$(dirname "$0")/_/husky.sh" | |||
|
|
|||
| npm run lint | |||
There was a problem hiding this comment.
Below we split out lint and test into two different steps. This aligns to our standard setup.
| @@ -21,7 +21,6 @@ | |||
| ], | |||
| "scripts": { | |||
| "lint": "eslint *.js", | |||
| "pretest": "npm run lint", | |||
There was a problem hiding this comment.
The precommit will still run both, but this aligns to our standard setup of individually explicit commands.
| @@ -8,6 +8,6 @@ To become a maintainer, create your own account on npm.js then ask the owners of | |||
|
|
|||
| To do this: | |||
| - After merging your changes, run `npm version -f major|minor|patch`. This will create a tagged commit changing the version in `package.json`, and the changes in `CHANGELOG.md`. | |||
| - Run `npm publish`. | |||
| - Run `npm publish --registry=https://registry.npmjs.org/`. | |||
There was a problem hiding this comment.
Be explicit, as not doing so can cause problems publishing to the wrong location.
| @@ -8,7 +8,7 @@ | |||
| }, | |||
| "settings": { | |||
| "react": { | |||
| "version": "16.4" | |||
| "version": "17.2" | |||
There was a problem hiding this comment.
I tried removing this locally to see what happens:
alakbarzeynalzade@M-LC2R24049H ~/dev/babel-plugin-transform-i18n add_gha ± npm run lint
> @skyscanner/[email protected] lint
> eslint *.js
Warning: React version was set to "detect" in eslint-plugin-react settings, but the "react" package is not installed. Assuming latest React version for linting.
| @@ -0,0 +1,29 @@ | |||
| name: PR | |||
There was a problem hiding this comment.
| @@ -1,12 +0,0 @@ | |||
| sudo: false | |||
When this repo was forked it had a
travis.ymlto run CI; but travis was never added to this repo.It had a pipeline for releases, but this was deleted when it stopped working, moving to manual releases. It was missing a pipeline for pull requests.
This PR ensures that linting and tests are enforced as a requirement for PRs. It does this using a simple GitHub Actions setup; as minimal changes as possible.
In addition to the code changes it also adds the
install-and-testas a required check in the repo settings.