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.

Sign up for GitHub

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

Jump to bottom

cpuset: init at 1.5.8 #48765

Merged
merged 1 commit into from Nov 8, 2018
Merged

cpuset: init at 1.5.8 #48765

merged 1 commit into from Nov 8, 2018

Conversation

wykurz
Copy link
Contributor

@wykurz wykurz commented Oct 21, 2018 •
edited

Motivation for this change

Add cpuset (cset) to nixpkgs.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

FRidh
FRidh requested changes Oct 21, 2018
View reviewed changes
pkgs/os-specific/linux/cpuset/default.nix Outdated
python3Packages.buildPythonApplication rec {
pname = "cpuset";
version = "1.6pre1";
name = "${pname}-${version}";
Copy link
Member

Choose a reason for hiding this comment

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

The name attribute is added by buildPython* and should therefore be removed.

pkgs/os-specific/linux/cpuset/default.nix Outdated

python3Packages.buildPythonApplication rec {
pname = "cpuset";
version = "1.6pre1";
Copy link
Member

Choose a reason for hiding this comment

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

we typically don't include pre-releases

@wykurz
Copy link
Contributor Author

wykurz commented Oct 21, 2018

Applied requested changes, please take a look @FRidh

@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

6 similar comments
@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

@FRidh
Copy link
Member

FRidh commented Oct 22, 2018

Good, commits need to be squashed.

@wykurz
Copy link
Contributor Author

wykurz commented Oct 23, 2018

@FRidh got it! ;) Done.

@wykurz
Copy link
Contributor Author

wykurz commented Oct 25, 2018

Hey @FRidh , could you take a look again? I believe all the requested fixes have been applied.

infinisil
infinisil requested changes Oct 27, 2018
View reviewed changes
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Also, you should split this PR up into one commit that adds you as a maintainer, and another one to add the package

pkgs/os-specific/linux/cpuset/default.nix Outdated Show resolved Hide resolved
@wykurz
Copy link
Contributor Author

wykurz commented Oct 27, 2018

@infinisil per your request to separate adding the maintainer into a new PR - is this documented? I might be missing something but I don't see this mentioned anywhere in:
https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md
https://nixos.org/nixpkgs/manual/#chap-submitting-changes

@infinisil
Copy link
Member

I guess this could be added to some document. The reason is because adding oneself as a maintainer (in maintainers-list.nix) has nothing to do with adding the package, and these should be separate commits because of that.

@wykurz
Copy link
Contributor Author

wykurz commented Oct 27, 2018 via email

@wykurz wykurz mentioned this pull request Oct 28, 2018
Merged
9 tasks
@wykurz wykurz changed the title cpuset: init at 1.6pre1 cpuset: init at 1.5.8 Oct 28, 2018
@wykurz
Copy link
Contributor Author

wykurz commented Oct 28, 2018

@infinisil updated with requested changes. Also updated the PR description and title.

infinisil
infinisil requested changes Oct 28, 2018
View reviewed changes
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@wykurz
Copy link
Contributor Author

wykurz commented Nov 4, 2018

@FRidh ping?

@infinisil
Copy link
Member

@GrahamcOfBorg build cpuset

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: cpuset

Partial log (click to expand)

reading manifest template 'MANIFEST.in'
warning: no files found matching 'ChangeLog'
writing manifest file 'cpuset.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/p3j35mlxldf1j9d5nsp1wvn1dpniwiii-cpuset-1.5.8

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: cpuset

Partial log (click to expand)

reading manifest template 'MANIFEST.in'
warning: no files found matching 'ChangeLog'
writing manifest file 'cpuset.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/kg1vap068h1i4262k4vf75hjkmgc3y6n-cpuset-1.5.8

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: cpuset

Partial log (click to expand)

reading manifest template 'MANIFEST.in'
warning: no files found matching 'ChangeLog'
writing manifest file 'cpuset.egg-info/SOURCES.txt'
running build_ext

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/nix/store/ldlb23lp4bpm323m4a818ji76n4hl68g-cpuset-1.5.8

@infinisil infinisil merged commit 2bdec61 into NixOS:master Nov 8, 2018
@wykurz wykurz deleted the cpuset branch October 12, 2019 04:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh requested changes

@infinisil infinisil infinisil approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@wykurz @FRidh @infinisil @GrahamcOfBorg