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

Jump to bottom

jump: init at 0.23.0 #60631

Merged
merged 1 commit into from May 6, 2019
Merged

jump: init at 0.23.0 #60631

merged 1 commit into from May 6, 2019

Conversation

sondr3
Copy link
Contributor

@sondr3 sondr3 commented May 1, 2019

Motivation for this change

Jump is a Go tool very similar to the well known z CLI tool with a few extras.

Things done

Installed and ran it on both my NixOS machine and my Mac, checked for shell completion and man pages and that it worked on both.

  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

sondr3
sondr3 commented May 1, 2019
View reviewed changes
pkgs/top-level/all-packages.nix
@@ -4127,6 +4127,8 @@ in

jumanpp = callPackage ../tools/text/jumanpp {};

jump = callPackage ../tools/system/jump {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not married to where I placed it, so if anyone has a better suggestion I'm all ears.

@ofborg ofborg bot requested a review from kalbasit May 1, 2019 12:09
tomberek
tomberek reviewed May 1, 2019
View reviewed changes
Copy link
Contributor

@tomberek tomberek left a comment

Choose a reason for hiding this comment

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

This seems to work without export HOME and I don't see a difference in the generated completion scripts.

@sondr3
Copy link
Contributor Author

sondr3 commented May 1, 2019 •
edited

I couldn't get it to work without it on my NixOS machine, I always got a bug: mkdir /homeless-shelter: permission denied error with it, how did you install it? Also, you are sure you're using the sandbox option when installing with Nix?

@tomberek
Copy link
Contributor

tomberek commented May 1, 2019

Yup, sandboxed builds. HOME is set to /homeless-shelter during build.

When does the error happen?

@sondr3
Copy link
Contributor Author

sondr3 commented May 1, 2019

Hmm, I'm not sure what my $HOME is when I build, I'm installing with nix-env -f . -i jump in my nixpkgs directory, are you doing something other than that? And it happens when I try to run the binary, it tries to access the configuration directory that jump uses I think and I get the error. If I uncomment any of those lines it works fine.

kalbasit
kalbasit requested changes May 1, 2019
View reviewed changes
pkgs/tools/system/jump/default.nix Outdated Show resolved Hide resolved
@kalbasit kalbasit self-assigned this May 1, 2019
@ofborg ofborg bot requested a review from kalbasit May 1, 2019 17:06
@kalbasit
Copy link
Member

kalbasit commented May 6, 2019

@GrahamcOfBorg build jump

@kalbasit kalbasit merged commit ffe0bb8 into NixOS:master May 6, 2019
@sondr3 sondr3 deleted the jump-init branch May 6, 2019 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomberek tomberek tomberek left review comments

@kalbasit kalbasit kalbasit approved these changes

Assignees

@kalbasit kalbasit

Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sondr3 @tomberek @kalbasit