Create test and lint Workflows
#8
Conversation
angel1254mc
requested review from
yonasbahre and
WillBAnders
and removed request for
yonasbahre
.github/workflows/lint.workflow.yml
Outdated
| on: | ||
| push: | ||
| branches: | ||
| - '*' |
There was a problem hiding this comment.
[Please Change]: For right now, we only want to run this on branches in PRs. It looks like there's an branches-ignore option that can be used instead for this, which should exclude the following:
master: There's probably no harm in it; but long-term it'd be nice to have a autodeploy action which will check forlint/testthere.gh-pages: This branch has our deployed application (which is not a node project) so I'm guessing the action would fail to run here as well.
There was a problem hiding this comment.
Switched from branches to branches-ignore in order to exclude master and gh-pages branches 😎
|
|
||
| steps: | ||
| - name: Checkout repo | ||
| uses: actions/checkout@v2 |
There was a problem hiding this comment.
[Question]: Vaguely familiar with GitHub actions. I was expecting to see something like setup-node here; is it not required? What happens without it?
There was a problem hiding this comment.
You're totally right, sorry for the oversight. I've gone ahead and added it in this latest commit /w Node 18 (LTS). Let me know if you think that's good 😄.
There was a problem hiding this comment.
LTS is good. FYI; if our project needed node itself we'd have it specified in package.json engines (and we'd want to match it to that) but since we don't this is first pick and LTS is the natural choice.
.github/workflows/lint.workflow.yml
Outdated
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Install dependencies | ||
| run: npm install |
There was a problem hiding this comment.
[Please Change]: Use npm ci (clean-install) instead - this verifies the versions used are those in package-lock.json and will fail if there's any discrepancies (rather than attempting to overwrite with new versions).
There was a problem hiding this comment.
Both workflows should now be using npm ci for installing dependencies :D.
.github/workflows/lint.workflow.yml
Outdated
| run: npm install | ||
|
|
||
| - name: Run lint | ||
| run: npm run lint |
There was a problem hiding this comment.
Just added a newline at the end of both files 👍.
| @@ -0,0 +1,17 @@ | |||
| on: | |||
There was a problem hiding this comment.
[Please Change]: Most of the notes above apply here too. For disabling this until we have npm run test setup; I think we can do that through the GitHub UI and that's fine for this.
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Set up Node |
There was a problem hiding this comment.
[Comment]: Should be after Checkout repo I believe.
There was a problem hiding this comment.
Went ahead and fixed that in this latest commit 👍
.github/workflows/lint.workflow.yml
Outdated
|
|
||
| - name: Run lint | ||
| run: npm run lint | ||
|
|
There was a problem hiding this comment.
Whoops; this has an extra space so the newline isn't applying.
There was a problem hiding this comment.
Thank you for catching that, I've gone ahead and remove the extra spaces at the end of that.
This PR adds two workflows, one for linting and one for testing. Testing workflow should be disabled until tests are added, otherwise it will yield an error due to the lack of a
testscript inpackage.json