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

nodejs_22: 22.4.1 -> 22.5.1 #327967

Merged
merged 2 commits into from
Jul 29, 2024
Merged

nodejs_22: 22.4.1 -> 22.5.1 #327967

merged 2 commits into from
Jul 29, 2024

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 17, 2024

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot requested a review from cillianderoiste July 17, 2024 18:54
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jul 17, 2024
@aduh95 aduh95 changed the title nodejs_22: 22.4.1 -> 22.5.0 nodejs_22: 22.4.1 -> 22.5.1 Jul 19, 2024
@vcunat
Copy link
Member

vcunat commented Jul 20, 2024

Why staging? As this version isn't the default one, the rebuild amount won't be high (for Hydra.nixos.org at least, by ofBorg as well).

@tie
Copy link
Member

tie commented Jul 21, 2024

Why staging?

I can’t speak for this particular PR, but I’d target staging for Node.js because changes usually cause mass rebuilds and we’ve seen some inconsistent master ↔ staging merges before (e.g. #325844 (comment)).

For example, if we run nixfmt on Node.js package with some minor manual cleanup on top of that, by this policy these changes should target master branch since there would be zero rebuilds. If, at the same time, staging branch contains other changes (e.g. another unrelated or possibly overlapping refactor), merging would require human intervention.

That is, since most of the Node.js package development happens on staging branch, I think targeting it for all changes reduces the likelihood of regressions and conflicts for staging merges. Changes take a bit longer to reach unstable release channel, but otherwise I’m not aware of any other issues with this approach.

@aduh95
Copy link
Contributor Author

aduh95 commented Jul 21, 2024

To add to what @tie said, if I have to add some changes that will impact the other Node.js versions (e.g. in this very PR, if the change suggested in #327967 (comment) lands upstream, it would have impact on all three versions of Node.js who are currently using use-correct-env-in-tests.patch), it's more convenient to not have to do the dance to change the base branch.

Copy link
Member

@tie tie left a comment

Choose a reason for hiding this comment

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

👍

@0x4A6F 0x4A6F merged commit d951858 into NixOS:staging Jul 29, 2024
30 checks passed
@aduh95 aduh95 deleted the 22.5.0-nodejs branch July 29, 2024 08:21
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