Skip to content
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

CI Improvements #17

Merged
merged 6 commits into from
Feb 11, 2022
Merged

CI Improvements #17

merged 6 commits into from
Feb 11, 2022

Conversation

s0
Copy link
Contributor

@s0 s0 commented Feb 11, 2022

Further to the CI integrations mentioned in #13 (prettier & eslint), this PR makes a few more improvements:

  • Also check that package-lock.json is unchanged when running NPM install in Node.js v16 (this will fail when packages are installed and this file isn't updated, or when the version in package.json is updated but package-lock.json is not updated to match.
  • Run TypeScript compilation checks to make sure that everything type-checks.
  • Run Push & PR checks regardless of branch (I was finding it frustrating trying to determine whether what I'd written was working correctly on my fork until I had done this, and I imagine many others will find the same).

Open to discussing any of the above of course :)

Results when invalid TS or package-lock.json are pushed can be found here: https://github.com/s0/nody-greeter/actions/runs/1830680405

@s0 s0 force-pushed the ci-improvements branch 2 times, most recently from 91fefee to d6e7de9 Compare February 11, 2022 17:26
@s0 s0 force-pushed the ci-improvements branch from d6e7de9 to 474c82d Compare February 11, 2022 17:32
Copy link
Owner

@JezerM JezerM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks nice~

Thanks for this PR!

.github/workflows/check.yml Outdated Show resolved Hide resolved
@s0 s0 force-pushed the ci-improvements branch from 474c82d to e8153cd Compare February 11, 2022 17:50
Copy link
Owner

@JezerM JezerM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice~

@JezerM JezerM merged commit 2b82df5 into JezerM:master Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants