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: Add 16.20.1 to permittedInsecurePackages in release.nix #239400

Closed
wants to merge 2 commits into from

Conversation

YorikSar
Copy link
Contributor

Description of changes

nodejs package has been updated in #238884 but it wasn't added here, so Hydra doesn't build any packages depending on it.

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.11 Release Notes (or backporting 23.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.

nodejs package has been updated in NixOS#238884
but it wasn't added here, so Hydra doesn't build any packages depending
on it.
@YorikSar
Copy link
Contributor Author

cc @marsam who updated nodejs.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Jun 23, 2023
@RaitoBezarius
Copy link
Member

Please drop the formatting changes, they are not necessary and would only hinder merging this PR.
Also, please do the same thing as in #238443 i.e. adding a comment in the package definition to prevent further issues.

cc @drupol who did the same PR in #239406.

@emilylange
Copy link
Member

The mentioned #239406 got merged 3 minutes ago.
Closing.

@emilylange emilylange closed this Jun 23, 2023
@emilylange
Copy link
Member

Whoops, I might have been a bit trigger happy with the close button, sorry.

Feel free to open a new PR for what @RaitoBezarius suggested

Also, please do the same thing as in #238443 i.e. adding a comment in the package definition to prevent further issues.

(if you want, no pressure)

@YorikSar YorikSar deleted the secure-nodejs-16 branch June 23, 2023 18:34
@YorikSar
Copy link
Contributor Author

@emilylange no problem, that PR adds that comment as well and doesn’t include formatting. Although I would prefer to format as much code as possible, it’s nothing critical.

@lilyinstarlight
Copy link
Member

lilyinstarlight commented Jun 23, 2023

Although I would prefer to format as much code as possible, it’s nothing critical.

I would love to be able to reformat a lot of the old stuff in nixpkgs, but until we have a blessed formatter, any formatting choices made towards that would be just as arbitrary as the original formatting. This is a pretty central file and it wasn't obvious how much of an improvement the formatting changes were, so it really just makes this more unclear for merge since we don't have much of a policy on formatting and it should have only been a one or two line change

We will be able to enforce formatting changes soon, at least, once RFC101 work is complete. While I'm not directly involved in the RFC, I've been vaguely keeping up with it and there's been a lot of great work on it (so hopefully it won't be much longer, maybe only a couple of months)

Edit: Also, I know the other PR ended up getting merged, but thank you for your contribution regardless :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants