-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 package-lock.json #1375
Remove package-lock.json #1375
Conversation
We don't have any dependencies so this isn't as important to us, it's also ugly to keep seeing these in PRs.
The package-lock.json file also handles dev dependencies. It would be great if anyone developing xterm.js had the same dev dependencies installed, such as tslint. This also affects the CI builds. |
@amejia1 seems like a lot of hassle/noise maintaining this file to fix a problem that's never come up in the development of the library before. I'd prefer to pin the devDependencies to specific versions than keep this, but still that's overkill for such a relatively simple build process imo. |
Let's get rid of it. It only causes confusion and I've never seen a problem without it. IMHO the only case where this file is useful is to record and restore a dependency state for a software release. But since we don't have any dependencies in our releases we can skip this file. |
@Tyriar The package-lock.json file covers pinning the entire (dev)dependency tree as well (dependency's dependencies and so on). @mofux It is indeed useful for reproducing the dependency tree for other devs and for the CI builds. Maybe development of xterm.js has worked fine without it before, but your dependencies are still being updated. The proposed use of prebuild and prebuild-install in node-pty may have an impact now or in some future release of those two packages. The '--include' option in prebuild to include other file types (exe, dll, so on) was only recently added for example. That's just one example. I think explicitly defining and using the dependency tree for the sake of avoiding the "works on my dev environment" issues outweighs the inconvenience of seeing it in PRs. |
@amejia1 oh I understand that, but since we're not actually shipping any dependencies this could only affect us for dev dependencies. And I would argue that the chance it affects us at all is not worth the maintenance burden of keeping the package-lock. If we did ship dependencies I would advocate for the opposite.
AFAIK we have never had such an issue in the ~2 years I've been involved in the project. Requesting more feedback from @xtermjs/core |
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.
🙏 let's get rid of this. package-lock.json
has never been useful for this particular project or my personal work.
If it turns out people have hard time developing xterm.js, we can search for a solution at that time. Also, even in that case I am pretty confident that Yarn will make the cut instead.
We don't have any dependencies so this isn't as important to us, it's also ugly
to keep seeing these in PRs.