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

vsh: init at 0.7.2 #99565

Open
wants to merge 1 commit into
base: master
from
Open

vsh: init at 0.7.2 #99565

wants to merge 1 commit into from

Conversation

@fishi0x01
Copy link
Member

@fishi0x01 fishi0x01 commented Oct 4, 2020

Motivation for this change

Add vsh tool

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.
pkgs/tools/misc/vsh/default.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Nov 28, 2020

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

1 package built:
  • vsh
@fishi0x01 fishi0x01 force-pushed the fishi0x01:fishi0x01/vsh branch from 410d1ba to 4793de4 Nov 28, 2020
@fishi0x01 fishi0x01 requested a review from SuperSandro2000 Nov 28, 2020
homepage = https://github.com/fishi0x01/vsh;
license = licenses.mit;
maintainers = with maintainers; [ fishi0x01 ];
platforms = platforms.linux ++ platforms.darwin;

This comment has been minimized.

@SuperSandro2000

SuperSandro2000 Nov 28, 2020
Contributor

Missed that the first round. That is the default for buildGoModule and must only be set if it is smaller. Eg only linux or darwin.

This comment has been minimized.

@fishi0x01

fishi0x01 Nov 28, 2020
Author Member

Fixed 👍

@SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Nov 28, 2020

Result of nixpkgs-review pr 99565 run on x86_64-darwin 1

1 package built:
  • vsh
@fishi0x01 fishi0x01 force-pushed the fishi0x01:fishi0x01/vsh branch from 4793de4 to 05186a6 Nov 28, 2020
@fishi0x01 fishi0x01 requested a review from SuperSandro2000 Nov 28, 2020
# make sure version gets set at compile time
buildPhase = ''
go install -ldflags "-X main.vshVersion=v${version}"
'';
Comment on lines +17 to +20

This comment has been minimized.

@SuperSandro2000

SuperSandro2000 Nov 28, 2020
Contributor

Suggested change
# make sure version gets set at compile time
buildPhase = ''
go install -ldflags "-X main.vshVersion=v${version}"
'';
buildFlagsArray = [ "-ldflags=-s -w -X main.vshVersion=v${version}" ];

According to other go packages this should be done like.

This comment has been minimized.

@fishi0x01

fishi0x01 Nov 28, 2020
Author Member

Which other packages are you referring to? Any examples?

This comment has been minimized.

@fishi0x01

fishi0x01 Nov 28, 2020
Author Member

Further, is there any official conventions on this?


meta = with lib; {
description = "HashiCorp Vault interactive shell";
homepage = https://github.com/fishi0x01/vsh;

This comment has been minimized.

@SuperSandro2000

SuperSandro2000 Nov 28, 2020
Contributor

Suggested change
homepage = https://github.com/fishi0x01/vsh;
homepage = "https://github.com/fishi0x01/vsh";

This comment has been minimized.

@fishi0x01

fishi0x01 Nov 28, 2020
Author Member

Done

@fishi0x01 fishi0x01 force-pushed the fishi0x01:fishi0x01/vsh branch from 05186a6 to 145bce3 Nov 28, 2020
@fishi0x01 fishi0x01 requested a review from SuperSandro2000 Nov 28, 2020
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.