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

Update and pin dependencies #770

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Conversation

tholu
Copy link
Contributor

@tholu tholu commented Nov 18, 2022

I have updated the dependencies to the latest available version and pinned them, which I believe to be a best practice in dependency management.

@tholu
Copy link
Contributor Author

tholu commented Nov 18, 2022

Not sure why it failed. There is a 401 error in the Task log. Tests run fine locally.

@gijsk
Copy link
Contributor

gijsk commented Nov 18, 2022

I'm asking around about the task log failure. I do have a separate curiosity question: why increase the nodejs version requirement, which presumably stops people still using older versions using an up-to-date version of this library? From a quick web search it seems Node 14 is still supported as LTS (though 12 isn't anymore).

@tholu
Copy link
Contributor Author

tholu commented Nov 18, 2022

@gijsk Thanks!

You're right, 14 is still maintained. I wasn't sure if it works with updated dependencies under 10 anymore and wanted to push people in the direction of a current LTS (which is 18 now). Happy to change it to 14 or keep it at 10 if there are no side-effects!

@gijsk
Copy link
Contributor

gijsk commented Nov 18, 2022

Matrix for taskcluster suggests that the reason the PR fails tests is:

gijs: the package-lock.json in the PR references https://nexus.symptoma.com/repository/npm-group/@babel/code-frame/-/code-frame-7.18.6.tgz which gives me 401

@tholu Perhaps this is an internal thing setup in your NPM setup that isn't externally accessible?

@gijsk
Copy link
Contributor

gijsk commented Nov 18, 2022

Looks like I can't push to your PR but I pastebin'd a diff with just deleting package-lock.json and regenerating it, which produced this: https://paste.mozilla.org/VyeUu6vT

I think just applying this should make things work? But my npm-fu is pretty weak, and I'm traveling so it's possible I'm missing some git/github options that would have made this easier.

@gijsk
Copy link
Contributor

gijsk commented Nov 18, 2022

(Oh, and I have no idea why that removes the SRI bits and some license bits from package-lock - maybe because I used --package-lock-only? It's not clear.)

@tholu
Copy link
Contributor Author

tholu commented Nov 18, 2022

@gijsk That makes sense, the registry in my local config was not the official one, so I fixed that now. Unclear to me why it strips the license bits - my guess is that newer npm versions strip it by default.

Copy link
Contributor

@gijsk gijsk left a comment

Choose a reason for hiding this comment

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

Thanks!

@gijsk gijsk merged commit fde3594 into mozilla:master Nov 18, 2022
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.

2 participants