Cwa-documentation: package-lock.json uses npm v6.x.x format - guidance in CONTRIBUTING.md needed

Created on 8 Jan 2021  路  4Comments  路  Source: corona-warn-app/cwa-documentation

Where to find the issue

CONTRIBUTING.md

Describe the issue

Running npm audit fix against https://github.com/corona-warn-app/cwa-documentation/blob/master/package-lock.json using the 7.3.0 version of npm bundled with the current version 15.5.1 of Node.js from https://nodejs.org/en/ produces a different format of the file https://github.com/corona-warn-app/cwa-documentation/blob/master/package-lock.json compared to the original version of the file.

The issue does not occur using the npm version 6.14.10 from the LTS (Long Term Support) version 14.15.4 of Node.js.

Suggested change

To avoid creating a different format of https://github.com/corona-warn-app/cwa-documentation/blob/master/package-lock.json and possibly risk other compatibility issues, CONTRIBUTING.md should contain instructions about the recommended version of Node.js and npm to install.

The website README.md includes some description about installing npm. This could perhaps be used as a basis.

Additional information

Running the following command using npm 6.14.10 with a package-lock.json produced via npm audit fix under npm 7.4.0 throws a warning.

$ npm install
npm WARN read-shrinkwrap 
This version of npm is compatible with lockfileVersion@1, 
but package-lock.json was generated for lockfileVersion@2. 
I'll try to do my best with it!
bug documentation in review

Most helpful comment

The problem with npm's lock file is that it is not treated as a lockfile which makes the whole thing complicated. In general, the documentation should recommend the version that the CI uses for testing (i.e. the package version distributed in Ubuntu).


Corona-Warn-App Open Source Team

All 4 comments

The problem with npm's lock file is that it is not treated as a lockfile which makes the whole thing complicated. In general, the documentation should recommend the version that the CI uses for testing (i.e. the package version distributed in Ubuntu).


Corona-Warn-App Open Source Team

@heinezen

In general, the documentation should recommend the version that the CI uses for testing (i.e. the package version distributed in Ubuntu).

Good point!
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu1804-README.md and
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md both specify

Npm 6.14.9 6.14.11

(6.14.11 is currently the latest LTS version for ubuntu , so 6.14.9 is very close.)

@MikeMcC399 I think we can close this issue here without any changes to the repo because unlike in https://github.com/corona-warn-app/cwa-website/issues/763, this repo uses npm only for the CI which shouldn't be touched by outside contributors.


Corona-Warn-App Open Source Team

@heinezen

I agree to close this issue.

There are no instructions in this repository to install npm, so there are no instructions to correct or clarify.

After the resolution of https://github.com/corona-warn-app/cwa-documentation/issues/495 concerning the CI work flow, any checks which use npm are carried out automatically without the need to have npm installed locally, also for contributions from forks.

I would still run npm test locally before committing and pushing, but that is a personal preference.

Thanks for considering (and for updating the README in the cwa-website repo)!

Was this page helpful?
0 / 5 - 0 ratings