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

Add package-lock.json to source control #1393

Merged
merged 1 commit into from
May 15, 2023
Merged

Conversation

jotaen4tinypilot
Copy link
Contributor

@jotaen4tinypilot jotaen4tinypilot commented May 15, 2023

While reviewing #1390, I realized that we haven’t checked in package-lock.json into version control, but we explicitly ignore it currently.

package-lock.json is an auto-generated manifest file which is created and maintained by npm on every change to package.json. Having the lock file under version control is actually recommended, because in contrast to the pure package.json file, the lock file captures the entire dependency tree. This aims to make builds more reproducible, faster, and safer (due to checksums).

Note: if git commit statistics are important to us, then we can also commit this under a separate account (e.g. some sort of bot account), since package-lock.json changes are always relatively large diffs in terms of LOC added/deleted.
Review on CodeApprove

Copy link
Contributor

@mtlynch mtlynch left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved on CodeApprove
✔️ Approved

LGTM

I think the gitignore rule was maybe before we were including package.json but I was still using tooling or something? But I agree we should be keeping it in source.


👀 @jotaen4tinypilot it's your turn please take a look

Copy link
Contributor Author

@jotaen4tinypilot jotaen4tinypilot left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: Discussion
Yes, it seems this was from a time where we installed Prettier on the fly during the CI build, and we didn’t have a package.json at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants