Skip to content

shell.nix: remove nixpkgs-review#373945

Closed
paparodeo wants to merge 1 commit intoNixOS:masterfrom
paparodeo:no-nixpkgs-revew-in-shell.nix
Closed

shell.nix: remove nixpkgs-review#373945
paparodeo wants to merge 1 commit intoNixOS:masterfrom
paparodeo:no-nixpkgs-revew-in-shell.nix

Conversation

@paparodeo
Copy link
Contributor

current version of nixpkgs-review is 3.0.1 but the pinned version in shell.nix is 2.12.0. This version doesn't work with the new CI and seems to just post empty results when used with a github token.A

unlike nixfmt, nixpkgs-review doesn't need to be pinned

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

current version of nixpkgs-review is 3.0.1 but the pinned version in
`shell.nix` is 2.12.0. This version doesn't work with the new CI and
seems to just post empty results when used with a github token.A

unlike nixfmt, nixpkgs-review doesn't need to be pinned
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 15, 2025
@paparodeo
Copy link
Contributor Author

cc @Scrumplex

@Scrumplex
Copy link
Member

but the pinned version in shell.nix is 2.12.0.

In that case, why don't we just update the version pin? From what I can tell, we only pin the version to avoid rebuilds of the shell during development.

@paparodeo
Copy link
Contributor Author

but the pinned version in shell.nix is 2.12.0.

In that case, why don't we just update the version pin? From what I can tell, we only pin the version to avoid rebuilds of the shell during development.

either way -- remove it or make it work. the current situation is not great. if no one is going to be maintaining it to keep it up to date it should be removed.

@paparodeo
Copy link
Contributor Author

paparodeo commented Jan 17, 2025

maybe pull nixpkgs-review from curPkgs ? [edit] except this is problematic on staging branches

diff --git a/shell.nix b/shell.nix
index ecb444e75ec0..18f5a60eca8b 100644
--- a/shell.nix
+++ b/shell.nix
@@ -28,12 +28,13 @@ let
 in
 curPkgs
 // pkgs.mkShellNoCC {
-  packages = with pkgs; [
+  packages = (with pkgs; [
     # The default formatter for Nix code
     # See https://github.com/NixOS/nixfmt
     nixfmt-rfc-style
+  ]) ++ (with curPkgs; [
     # Helper to review Nixpkgs PRs
     # See CONTRIBUTING.md
     nixpkgs-review
-  ];
+  ]);
 }

?

@philiptaron
Copy link
Contributor

Can we update the pin for CI instead, so that it encompasses dc208d2 (nixpkgs-review: 3.0.0 -> 3.0.1)?

Have there been breaking changes on the nixfmt-rfc-style that would prevent that? I don't see any, based on commits to pkgs/by-name/ni/nixfmt-rfc-style/.

@paparodeo
Copy link
Contributor Author

having a broken tools seems ok.

@paparodeo paparodeo closed this Jan 22, 2025
@paparodeo paparodeo deleted the no-nixpkgs-revew-in-shell.nix branch January 22, 2025 21:36
@philiptaron
Copy link
Contributor

I think this has now been remedied in #376039

Thanks @Scrumplex

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 on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants