Skip to content

jetbrains: replace overrideDerivation with overrideAttr#107344

Merged
jonringer merged 1 commit intoNixOS:masterfrom
raboof:jetbrains-replace-overrideDerivation-with-overrideAttr
Feb 17, 2021
Merged

jetbrains: replace overrideDerivation with overrideAttr#107344
jonringer merged 1 commit intoNixOS:masterfrom
raboof:jetbrains-replace-overrideDerivation-with-overrideAttr

Conversation

@raboof
Copy link
Copy Markdown
Member

@raboof raboof commented Dec 21, 2020

Motivation for this change

This fixes error-checking of function arguments (
https://nixos.org/manual/nixpkgs/stable/#sec-pkg-overrideDerivation),
notably checking the license (#107250)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@raboof raboof requested a review from edwtjo as a code owner December 21, 2020 20:39
@raboof raboof force-pushed the jetbrains-replace-overrideDerivation-with-overrideAttr branch from 4c62759 to e36c68e Compare December 21, 2020 21:00
@fabianhjr fabianhjr mentioned this pull request Dec 21, 2020
Merged
@raboof raboof force-pushed the jetbrains-replace-overrideDerivation-with-overrideAttr branch from e36c68e to e794c8c Compare January 4, 2021 12:43
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 4, 2021
@raboof raboof force-pushed the jetbrains-replace-overrideDerivation-with-overrideAttr branch from e794c8c to 75641c0 Compare January 21, 2021 18:37
@ofborg ofborg 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. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jan 21, 2021
@nixos-discourse
Copy link
Copy Markdown

nixos-discourse commented Jan 21, 2021

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/447

@SuperSandro2000
Copy link
Copy Markdown
Member

SuperSandro2000 commented Jan 22, 2021

ping @raboof @edwtjo @lovek323

@raboof
Copy link
Copy Markdown
Member Author

raboof commented Jan 22, 2021

(I created this PR, so naturally I'm in favor ;) )

hugolgst
hugolgst approved these changes Jan 22, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@hugolgst hugolgst left a comment

Choose a reason for hiding this comment

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

Code-wise it looks good to me

@SuperSandro2000
Copy link
Copy Markdown
Member

SuperSandro2000 commented Jan 22, 2021

(I created this PR, so naturally I'm in favor ;) )

I was looking for another maintainer which can confirm that this works and does not break anything.

@raboof
Copy link
Copy Markdown
Member Author

raboof commented Jan 29, 2021

Result of nixpkgs-review pr 107344 run on x86_64-linux 1

jonringer
jonringer approved these changes Feb 17, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

was able to build and launch clion.

@raboof raboof mentioned this pull request Jun 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edwtjo edwtjo Awaiting requested review from edwtjo

2 more reviewers

@jonringer jonringer jonringer approved these changes

@hugolgst hugolgst hugolgst approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@raboof @nixos-discourse @SuperSandro2000 @jonringer @hugolgst