-
Notifications
You must be signed in to change notification settings - Fork 56
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
Remove yarn config files #156
Conversation
yarn is no longer used in CI, so yarn-specific is no longer required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having some kind of lockfile is useful for developers. (Not to mention… CI.) Translate to package-lock.json?
Added in #157. |
As noted in #157, lockfiles are useful by default for applications but intentionally not used for most libraries. npm will obey a lockfile in a dependency. We want to test with the latest matching dependency according to the range in the package, not lock. Nor do we want to lock all the dependencies and have to release an update to this package on each new dependency version. There's a tradeoff here and it's not necessarily wrong to lock library dependencies. But it's not typically the npm way and especially inconvenient for a meta package like this that wires up several smaller packages for complex types. |
It sounds like you’re referring to npm-shrinkwrap.json. yarn.lock/package-lock.json aren’t published; in a library, they serve to make the full development state for a given revision reproducible. |
Ah right:
Still, the point stands that it doesn't seem lockfiles are actually solving a problem here, and more likely creating one. |
They solve the problem I mentioned (which pg has been affected by, notably), and their absence isn’t really a solution to the problem sindresorhus mentioned back in 2017 (dependabot is more reliable). Almost the opposite, really: if you run Anyway, not that big a deal one way or another for this repo – I’m used to reconstructing dependency trees for a point in time by now. Just wanted to get accurate information out there. |
Right, the GHA addition is overdue and very valuable. I understand that lockfile benefits apply to library developers. Unlike applications, there's a real tradeoff to be considered. I'm unpersuaded that libraries having lockfiles is the right default. Tooling has improved since in the last 5 years but the need to configure and operate it is still extra work. Definitely not something to sneak in alongside other changes. If someone wants to do the work to run tests both with and without a lockfile in GHA I'm happy to review that PR. |
The lockfile was introduced in #152, and mentioned clearly in the PR description. A build without
|
"Sneak" is the wrong word, but ultimately there was an unrelated change (using Yarn) proposed that didn't need to be, which then necessitated another unrelated change (a lockfile). The GHA workflow is an unqualified improvement, thank you! There's more discussion/work needed on the lockfile and I don't have the time to devote to that right now. |
yarn is no longer used in CI, so yarn-specific config files are no longer required.