Skip to content

Conversation

@anthonyroussel
Copy link
Member

@anthonyroussel anthonyroussel commented Jan 8, 2023

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@anthonyroussel anthonyroussel requested a review from marsam January 8, 2023 22:08
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. labels Jan 8, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1686

@marsam
Copy link
Contributor

marsam commented Jan 11, 2023

Thank you! Please target it to staging-next

@anthonyroussel anthonyroussel changed the base branch from master to staging-next January 12, 2023 12:09
@anthonyroussel
Copy link
Member Author

Thank you! Please target it to staging-next

@marsam Thanks for the review. I retargeted the PR to staging-next.

@anthonyroussel
Copy link
Member Author

Fixed the failing build for nodejs-19_x, cause was patches in pkgs/development/web/nodejs/npm-patches.nix are now useless for nodejs-19_x because they are merged upstream.

They are still required for NodeJS 14, 16, 18 however.

@anthonyroussel
Copy link
Member Author

Rebased my branch against staging-next.

Copy link
Contributor

@marsam marsam left a comment

Choose a reason for hiding this comment

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

Thank you!

@marsam marsam merged commit 374d741 into NixOS:staging-next Jan 14, 2023
@anthonyroussel anthonyroussel deleted the nodejs branch January 14, 2023 09:15
@winterqt
Copy link
Member

Hi @marsam, just checking: do you remember why you sent this directly into an active -next? It doesn't seem to be security related. Thanks!

@marsam
Copy link
Contributor

marsam commented Feb 22, 2023

Hi, I suggested to send it to staging-next because Node v19.3.0 updated npm to 9.2.0, and staging-next had npmPatches which were backports of npm 9.
If this PR landed to master first, it would break nodejs-19_x when staging-next is merged since it merges cleanly, even though npmPatches were conflicting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants