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

Replace npm with yarn; add CI workflow to run TSC and tslint checks #699

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

laurence-myers
Copy link
Contributor

To avoid linting issue sneaking in, particularly in the backend/server code, I've added a GitHub Action workflow to run tsc and tslint on every push.

yarn behaves better across different execution environments. In particular, npm ci failed to run in a GitHub Action, due to fsevents being a required dependency (somehow).

Example CI check:

https://github.com/laurence-myers/Video-Hub-App/runs/3578230215?check_suite_focus=true

This PR is based on #698, and includes its changes.

@whyboris
Copy link
Owner

whyboris commented Sep 12, 2021

Thank you for the contribution! I'll give it a try on Mac and Windows to see how it goes.

I remember when yarn was really ahead of npm in many ways, but it has felt like npm has caught up in terms of speed and locking dependencies 🤔

I'm not opposed to merging this -- I'll try it out first 🙇

@laurence-myers
Copy link
Contributor Author

Thank you for the contribution! I'll give it a try on Mac and Windows to see how it goes.

I remember when yarn was really ahead of npm in many ways, but it has felt like npm has caught up in terms of speed and locking dependencies 🤔

I'm not opposed to merging this -- I'll try it out first 🙇

Yep, I'd be happy to stick with npm, I just didn't know if it would be possible to fix the issue where package-lock.json lists fsevents as a required (transitive) dependency. Maybe using resolutions in package.json?

Plus, I noticed package-lock.json has a bunch of possibly invalid dependencies beginning with node_modules. I ran npm shrinkwrap, which gave a much smaller npm-shrinkwrap.json file.

If you look at the number of lines changed, this MR removes approx 30,000 lines. Maybe it's because npm-shrinkwrap.json is more thorough in how it lists dependencies, but maybe it's because there's unnecessary/invalid entries.

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.

None yet

2 participants