Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Update recommendations for lockfiles #25

Merged
merged 28 commits into from Jul 13, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions review/npm.md
Expand Up @@ -287,7 +287,7 @@ unpinned dependencies and update the lockfiles:
being installed at build-time in your CI or otherwise automated environment.

1. Developers should declare and commit a lockfile for ***all*** their
projects. The reasoning is that this lockfile will provide of
projects. The reasoning is that this lockfile will provide the benefits of
[Reproducible installation](#reproducible-installation)
by default for privileged environments (project developers' machines,
CI, production or other environments with access to sensitive data,
Expand All @@ -309,9 +309,13 @@ unpinned dependencies and update the lockfiles:
calculator](https://semver.npmjs.com/) to help you define the ranges.

1. The lockfile `package-lock.json` ***should*** be ignored for tests running in CI
(e.g. via `npm install --no-package-lock`). The reasoning is that developers should
(e.g. via `npm install --no-package-lock`). The reasoning is that CI tests should
Copy link
Member

Choose a reason for hiding this comment

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

to be fair tho, this is also true for developers, despite the (relatively small) risk to their local machines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't entirely disagree. I think developers will do in practice regardless of what we say (e.g. when troubleshooting/repdoducing a problem). I think the intention is to raise awareness and encourage users to run these non-locked tests in CI/less-privileged env when possible.

Let me know if you think we should change it or if this is acceptable as-is.

exercise a wide range of dependency versions in order to discover / fix problems
before their users do, so tests need to pull the latest versions of packages.
before the library users do, so tests need to pull the latest versions of packages.

1. Locally, developers should only run npm commands that treat the lockfile as
read-only (see [Lockfiles and commands](#lockfiles-and-commands)), except
when intentionally adding /removing a dependency.

1. Follow the principle of least privilege in your [CI configuration](#ci-configuration).
This is particularly important since the lockfile is ignored.
Expand Down