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

package-lock.json: To commit or not to commit? #4408

Closed
johnsaigle opened this issue Mar 20, 2019 · 8 comments
Closed

package-lock.json: To commit or not to commit? #4408

johnsaigle opened this issue Mar 20, 2019 · 8 comments
Assignees
Labels
Meta PR does something that organizes, upgrades, or manages the functionality of the codebase State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed

Comments

@johnsaigle
Copy link
Contributor

This file should either be committed explicitly to the repo or explicitly included in our .gitignore. Right now it floats around between branches and causes warnings when switching.

I'm not sure on the best practices here because I'm not really involved with JavaScript, but a decision should be made.

@johnsaigle johnsaigle added Meta PR does something that organizes, upgrades, or manages the functionality of the codebase State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed labels Mar 20, 2019
@johnsaigle
Copy link
Contributor Author

I've assigned our resident JS specialists to sort this out. 👯

@maltheism
Copy link
Member

@johnsaigle generally the best practice is to commit the package-lock.json. @HenriRabalais @zaliqarosli do you both agree or is there a reason we shouldn't in LORIS?

@maltheism
Copy link
Member

maltheism commented Mar 20, 2019

Also see stackoverflow answer.

@johnsaigle
Copy link
Contributor Author

Yeah I arrived at the same SO post but some people on that thread seem to have issues with it when doing merges etc. so 🤷‍♂️

@zaliqarosli
Copy link
Contributor

i don't have strong strong opinions but it seems like we should be committing it because 1. changes are only generated when node_modules or package.json are modified and 2. it must match package.json, so everytime package.json is committed, we should be committing package-lock.json alongside it. if the file exists on github, then we may as well be committing it. i think the issue right now is that the two files are out of sync with package-lock not getting updated when package was

Screenshot 2019-03-20 13 12 30

@zaliqarosli
Copy link
Contributor

@johnsaigle
Copy link
Contributor Author

@zaliqarosli Yeah I think you're right that they're out of sync. Also package-lock.json appears to be only on major.

@maltheism
Copy link
Member

Both @driusan and @xlecours are the only ones who have ever committed the file!

driusan pushed a commit that referenced this issue Apr 1, 2019
Bring package-lock.json in sync with package.json. At some point the latter was changed without the former being committed to the repo.

Resolves #4408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta PR does something that organizes, upgrades, or manages the functionality of the codebase State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed
Projects
None yet
Development

No branches or pull requests

4 participants