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

pipr: init at 0.0.12 #94810

Merged
merged 2 commits into from Aug 18, 2020
Merged

pipr: init at 0.0.12 #94810

merged 2 commits into from Aug 18, 2020

Conversation

@elkowar
Copy link
Contributor

elkowar commented Aug 6, 2020

Motivation for this change

https://github.com/elkowar/pipr

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.
@mweinelt
Copy link
Member

mweinelt commented Aug 7, 2020

I'm getting the following error, so something with bubblewrap is not working out.

[nix-shell:~/.cache/nixpkgs-review/pr-94810]$ ./results/pipr/bin/pipr
bubblewrap installation not found. Please make sure you have `bwrap` on your path, or supply --no-isolation to disable safe-mode

Result of nixpkgs-review pr 94810 1

1 package built:
- pipr
Copy link
Member

mweinelt left a comment

Thanks for your submission, I've added a few remarks.

@@ -2448,6 +2448,12 @@
githubId = 97852;
name = "Ellis Whitehead";
};
elkowar = {

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

Please move this change into a separate commit.


rustPlatform.buildRustPackage rec {
pname = "pipr";
version = "v0.0.12";

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

Suggested change
version = "v0.0.12";
version = "0.0.12";
src = fetchFromGitHub {
owner = "ElKowar";
repo = pname;
rev = "v0.0.12";

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

Suggested change
rev = "v0.0.12";
rev = "v${version}";
@@ -22758,6 +22758,8 @@ in

synergyWithoutGUI = synergy.override { withGUI = false; };

pipr = callPackage ../applications/misc/pipr { };

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

The file is somehwat sorted alphabetically, can you please try and find a better spot to put this?

description = "A commandline-tool to interactively write shell pipelines";
homepage = "https://github.com/ElKowar/pipr";
license = licenses.mit;
maintainers = maintainers.elkowar;

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

Suggested change
maintainers = maintainers.elkowar;
maintainers = with maintainers; [ elkowar ];

This should be a list.

pname = "pipr";
version = "v0.0.12";

src = fetchFromGitHub {

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

I think it would be better if we used the upstream, not the mirror, don't you? fetchGit + https://gitlab.com/Elkowar/pipr

This comment has been minimized.

@elkowar

elkowar Aug 7, 2020 Author Contributor

generally yes. In this case, I'm planning on moving my projects over to github completely in the near future, so i makes sense to directly use github. If using a mirror in general is an issue, I can use gitlab here of course

This comment has been minimized.

@mweinelt

mweinelt Aug 7, 2020 Member

Whatever works for you.

@elkowar elkowar force-pushed the elkowar:new_package_pipr branch from 90dbba3 to 784018b Aug 8, 2020
@elkowar
Copy link
Contributor Author

elkowar commented Aug 8, 2020

I updated the PR, everything you mentioned should be fixed now, @mweinelt
The documentation regarding how to contribute to nixpkgs mentions that the all-packages.nix file is sorted by category and then alphabetically. If, apparently, this has changed to now just be fully sorted alphabetically, the documentation should probably be updated in that regard

@elkowar elkowar force-pushed the elkowar:new_package_pipr branch from 784018b to 416cfc7 Aug 18, 2020
@mweinelt
Copy link
Member

mweinelt commented Aug 18, 2020

Result of nixpkgs-review pr 94810 1

1 package built:
- pipr
@mweinelt mweinelt merged commit dd03272 into NixOS:master Aug 18, 2020
@mweinelt
Copy link
Member

mweinelt commented Aug 18, 2020

Sorry, took me a while to come back here.

@elkowar
Copy link
Contributor Author

elkowar commented Aug 18, 2020

no worries, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.