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

chore: package-lock changes due to npm install (node v14.21.3) #350

Merged
merged 3 commits into from
Apr 13, 2024

Conversation

cameronhunter
Copy link
Contributor

Description

npm install made changes to package-lock.json.

Motivation and Context

When following the CONTRIBUTING guide, I ran npm install using the latest LTS version of node (v20). npm warned me about an old lockfile version and that it would rewrite it.

$ npm install
npm WARN old lockfile 
npm WARN old lockfile The package-lock.json file was created with an old version of npm,
npm WARN old lockfile so supplemental metadata must be fetched from the registry.
npm WARN old lockfile 
npm WARN old lockfile This is a one-time fix-up, please be patient...
npm WARN old lockfile 

However, the lockfile-lint dependency is not compatible with the latest lockfile version. I tried again with node v18 and v16 but it had the same issue. Finally, node v14 does not attempt to rewrite the package-lock.json file and works with lockfile-lint, however, it does still make a small change to the file.

I don't see any documentation on what version of node should be used with the repository – is there a recommendation?

How Has This Been Tested?

npm run test

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • My changes are in sync with the code style of this project.
  • There aren't any other open Pull Requests for the same issue/update.
  • These changes should be applied to a maintenance branch.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using Jest-Image-Snapshot?

Ensure to use the version of node specified in the documentation and .nvmrc file.

@10xLaCroixDrinker
Copy link
Member

Thanks for the contribution @cameronhunter!

It looks like you've accidentally created a release in your branch? (version bump and changelog update)

I'd rather get the repo updated to be running on Node.js 18 & 20 than put more into Node.js 14.

@cameronhunter
Copy link
Contributor Author

@10xLaCroixDrinker I've rebased my changes which removed the version bump and changelog updates. It's been a while since I created this pull request so I can't remember if I made those changes or if they were automatic.

w.r.t. updating to node 18 and 20 – I completely agree. I wanted to make minimal changes to the repo to unblock some feature development. The repo is highly opinionated and I didn't want to make decisions about removing lockfile-lint.

@10xLaCroixDrinker 10xLaCroixDrinker merged commit da49e57 into americanexpress:main Apr 13, 2024
10 checks passed
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.

3 participants