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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: bump all dependencies #306

Merged
merged 3 commits into from
Apr 24, 2022
Merged

chore: bump all dependencies #306

merged 3 commits into from
Apr 24, 2022

Conversation

gamemaker1
Copy link
Member

@gamemaker1 gamemaker1 commented Apr 24, 2022

Related Issues

--

What Does This PR Do?

Changed

  • Bumped dependencies
  • Removed ^ and pinned them so that the npm doesn't automatically bump versions unless we ask it too.

Caveats/Problems/Issues

The library test on MacOS (Node 12) is failing because of a 409 error while fetching packages from the npm registry. Re-running the CI worked the second time, must be a network issue 馃し馃従

Checklist

  • The issues that this PR fixes/closes have been mentioned above.
  • What this PR adds/changes/removes has been explained.
  • All tests (npm test) pass.
  • All added/modified code has been commented, and
    methods/classes/constants/types have been annotated with TSDoc comments.
  • If a new feature has been added or a bug has been fixed, tests have been
    added for the same.


# Uses the exact version instead of any within-patch-range version of an
# installed package
save-exact=true
Copy link
Member

Choose a reason for hiding this comment

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

What's your thinking here? (We're already doing npm ci for our tests, which installs exact dependencies and sub-dependencies from package-lock.json.)

Copy link
Member

Choose a reason for hiding this comment

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

oh, actually, we're doing it for the top-level, but not the external tests. But still, we could change that if e wanted to.

Copy link
Member Author

Choose a reason for hiding this comment

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

npm ci is only for the tests, however, when someone new who wants to contribute runs npm install, npm will install any version within the patch range instead of exactly that version, and the person will commit the version bump. To prevent this, the save-exact removes the ^ from package.json.

A couple of tests fail on node 12 on some version of jest and typescript when I accidently bumped deps like this a week ago, hence I put this in to prevent it from happening in the future.

Copy link
Member

Choose a reason for hiding this comment

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

Eh, I use npm ci locally, but maybe I'm just weird 馃檭

Does save-exact prevent changes to package-lock.json when running npm install?

Fair enough, though. And, if current versions of jest or whatever else we depend on no longer work in node.js 12, then perhaps we should drop it from CI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Does save-exact prevent changes to package-lock.json when running npm install?

I think it does, take a look at some of the answers here

Fair enough, though. And, if current versions of jest or whatever else we depend on no longer work in node.js 12, then perhaps we should drop it from CI.

I haven't spend much time on it so I'm not exactly sure what is causing the trouble, will look into it and let you know

@gamemaker1 gamemaker1 merged commit 2b91eba into master Apr 24, 2022
@gamemaker1 gamemaker1 deleted the meta/bump-dependencies branch April 24, 2022 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants