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

build(deps): bump NPM from 9.5.1 to 9.8.1 #7659

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

THETCR
Copy link
Contributor

@THETCR THETCR commented Jul 28, 2023

There have been some serious bug fixes in later minor and patch versions of NPM v9.

Diff: v9.5.1...v9.8.1
Changelog

@THETCR THETCR requested a review from a team as a code owner July 28, 2023 11:51
@deivid-rodriguez
Copy link
Contributor

deivid-rodriguez commented Aug 23, 2023

Thanks for this, and sorry for not getting to it earlier 🙏.

Turns out we now have duplicated PRs for this: #7811.

That other PR has progressed a bit more. It has limited the upgrade to 9.6.7, since going further requires to drop old nodejs support.

It still has a spec failure, so I guess if you plan to work on this, you should coordinate with @yeikel to not duplicate efforts. Also make sure to add attribution to one another so both of you are credited through commit authorship!

@THETCR
Copy link
Contributor Author

THETCR commented Aug 31, 2023

@deivid-rodriguez Why would it require dropping old NodeJS support?
NPM just had a breaking change in their output, which should not have happened according to semantic versioning.

@jeffwidman
Copy link
Member

Re-opening as the other PR doesn't actually bump as high as this one does... it shouldn't have been cross-linked.

Sorry about that @THETCR !

Regarding the discussion on NodeJS, I'm afraid I don't have context and unfortunately can't spend the time right now to come up to speed on this as I'm about to step off the :dependabot: team for a bit.

_deivid-rodriguez just headed offline for several weeks on holiday (I think he might be 🚴 ?) so if no one on the team gets a chance to pick this up in the next few weeks, feel free to ping him again on/after Sept 20th.

@jeffwidman jeffwidman reopened this Aug 31, 2023
@yeikel
Copy link
Contributor

yeikel commented Aug 31, 2023

@deivid-rodriguez Why would it require dropping old NodeJS support? NPM just had a breaking change in their output, which should not have happened according to semantic versioning.

Do you know what is the process to select the version of npm that ships with NodeJS?

I ignore the reasons (I probably overlooked the documentation) but the latest build of Node 18(18.17.1 - 2023-08-08 ) shipped with NPM 9.6.7 and we had higher versions of NPM available. Is it just a matter of timings?

For safety, I think that it might be a good idea to keep them in sync as they are both tested together as part of the release but I could be wrong here if semantic releases are used

@yeikel
Copy link
Contributor

yeikel commented Aug 31, 2023

Re-opening as the other PR doesn't actually bump as high as this one does... it shouldn't have been cross-linked.

The reason it was cross-linked is because my PR initially bumped up to that version. I decided not to after we found the bug that I reported. This one can stay open or we can use the TODO as a reminder

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.

4 participants