kube3d: increase discoverability #99563

06kellyjac · 2020-10-04T14:44:53Z

Motivation for this change

I've searched for "k3d" previously using https://search.nixos.org but kube3d didn't show up so I've been maitaining my own copy of it for months 😅
Mentioning "k3d" in the description and longDescription should make it easier to find.

Things done

nixpkgs-fmted the file to fit with contributing guidelines.

Mentioned k3d in the description and longDescription

https://search.nixos.org can find things from either description but nix search just looks at the short description.

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)
- NixOS (x86_64)
- ~~[ ] macOS~~ Not on the platforms list
- 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.

nixos-discourse · 2020-10-22T09:27:00Z

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

06kellyjac · 2020-11-17T22:59:38Z

/marvin opt-in
/status needs_reviewer

marvin-mk2 · 2020-11-17T22:59:40Z

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

06kellyjac · 2020-11-17T23:00:14Z

Result of nixpkgs-review pr 99563 1

1 package built:

kube3d

jlesquembre · 2020-11-18T15:07:39Z

@06kellyjac thanks for the PR, I like your changes, but I have 2 questions:

Why did you convert buildFlagsArray to an array?
Why did you replaced stdenv.lib with lib?

About the first question, I found this related issue: #99200 but I still don't understand the reason.

I'm not opposed to do that changes, I just want to understand the motivation behind them

06kellyjac · 2020-11-18T15:36:23Z

Why did you convert buildFlagsArray to an array?

IMO it makes more sense for me that something called an "Array" actually be an array. I'm not sure if there are any other advantages or disadvantages that are more substantial.
I've never had any issues like the one you mentioned. Just from scanning it I'm not sure what's wrong with the concat output

λ BOB=("a" "b" "c")
λ echo "${BOB[@]}"
a b c

λ BUILDFLAGSARRAY=("-tags" "netgo" "-ldflags='-w -X PACKAGE.gitTag=VERSION'")
λ echo "${BUILDFLAGSARRAY[@]}" # notwrapped in 'quotes'
-tags netgo -ldflags='-w -X PACKAGE.gitTag=VERSION'

-tags netgo -ldflags='-w -X PACKAGE.gitTag=VERSION' looks fine to me, and at the very least it's working for k3d

Why did you replaced stdenv.lib with lib?

If stdenv is only being used for stdenv.lib then why not just grab lib directly like many other golang packages. If stdenv was being used for additional things it'd make sense to keep it like it was.

jlesquembre · 2020-11-18T17:32:49Z

IMO it makes more sense for me that something called an "Array" actually be an array.

Totally agree with that, it's only that in the examples I remember, it was always a string. And if it ain't broke, don't fix it.
But not idea if there is a recommended way for that, I didn't find anything in the docs.

If stdenv is only being used for stdenv.lib then why not just grab lib directly like many other golang packages.

It makes sense to me, I only wanted to know if there are some best practices about it.

To me, it's ok to merge your changes, I just wanted to know if there are some guide lines about that topics

06kellyjac · 2020-11-18T17:38:02Z

Thanks for looking it all over

I just wanted to know if there are some guide lines about that topics

No worries. I've not been using and packaging for nix too long so I'm also trying to work out some of the best practices 😅

marvin-mk2 · 2020-11-21T21:12:12Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR will be put back in the needs_reviewer queue in one day.

jlesquembre · 2020-11-23T14:37:55Z

I tested the executable and all looks good.
/status needs_merger

SuperSandro2000 · 2020-11-23T15:54:43Z

pkgs/applications/networking/cluster/kube3d/default.nix

    homepage = "https://github.com/rancher/k3d";
-    description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container";
+    description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container - k3d";


Suggested change

description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container - k3d";

description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container";

I added k3d to the short description because nix search uses description but doesn't look at longDescription

nix search seems to only look at the short description so it's worth
including there too.

06kellyjac@83cd754

I think this is fine with me. Sound good to leave it @SuperSandro2000?

kevincox · 2020-11-24T23:56:29Z

pkgs/applications/networking/cluster/kube3d/default.nix

    homepage = "https://github.com/rancher/k3d";
-    description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container";
+    description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container - k3d";


I think this is fine with me. Sound good to leave it @SuperSandro2000?

SuperSandro2000 · 2020-11-26T01:33:50Z

Can you please resolve the merge conflict?

search.nixos.org was able to show me `deno` when searching for "executable" which was in the longDescription

While I was able to find `deno` by searching for "executable" from it's longDescription on search.nixos.org, I couldn't find it using `nix search` `nix search` seems to only look at the short description so it's worth including there too.

Moved k3sVersion to be a variable. Converted buildFlagsArray to an array Moved vendorSha256 closer to src Moved doCheck between build and install related bits (like where the phase happens) Replaced stdenv.lib with lib

06kellyjac · 2020-11-26T12:41:24Z

Done

06kellyjac force-pushed the kube3d_discoverability branch from 0d6c1f3 to 3461263 Compare October 4, 2020 14:46

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Oct 4, 2020

06kellyjac force-pushed the kube3d_discoverability branch from 3461263 to 9b58a5a Compare November 17, 2020 22:55

marvin-mk2 bot added the marvin label Nov 17, 2020

marvin-mk2 bot added the needs_reviewer label Nov 17, 2020

ofborg bot requested review from kalbasit and kuznero November 17, 2020 23:45

marvin-mk2 bot removed the needs_reviewer label Nov 17, 2020

ofborg bot requested a review from jlesquembre November 17, 2020 23:45

marvin-mk2 bot added the awaiting_reviewer label Nov 17, 2020

ofborg bot added 11.by: package-maintainer 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-linux: 0 labels Nov 17, 2020

marvin-mk2 bot added needs_merger and removed awaiting_reviewer labels Nov 23, 2020

marvin-mk2 bot requested a review from kevincox November 23, 2020 14:38

marvin-mk2 bot added awaiting_merger and removed needs_merger labels Nov 23, 2020

SuperSandro2000 reviewed Nov 23, 2020

View reviewed changes

kevincox approved these changes Nov 24, 2020

View reviewed changes

06kellyjac added 4 commits November 26, 2020 12:36

kube3d: nixpkgs-fmt the file to fit contributing guidelines

c36b5d5

kube3d: add a longDescription to provide detail and help searches

7a03c0f

search.nixos.org was able to show me `deno` when searching for "executable" which was in the longDescription

kube3d: mention k3d in the short description

168df5f

While I was able to find `deno` by searching for "executable" from it's longDescription on search.nixos.org, I couldn't find it using `nix search` `nix search` seems to only look at the short description so it's worth including there too.

kube3d: re-order the derivation and general cleanup

cdfa9b0

Moved k3sVersion to be a variable. Converted buildFlagsArray to an array Moved vendorSha256 closer to src Moved doCheck between build and install related bits (like where the phase happens) Replaced stdenv.lib with lib

06kellyjac force-pushed the kube3d_discoverability branch from 9b58a5a to cdfa9b0 Compare November 26, 2020 12:40

marvin-mk2 bot added awaiting_reviewer and removed awaiting_merger labels Nov 26, 2020

kevincox merged commit a5da6e5 into NixOS:master Nov 26, 2020

06kellyjac deleted the kube3d_discoverability branch November 26, 2020 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kube3d: increase discoverability #99563

kube3d: increase discoverability #99563

06kellyjac commented Oct 4, 2020 •

edited

nixos-discourse commented Oct 22, 2020

06kellyjac commented Nov 17, 2020

marvin-mk2 bot commented Nov 17, 2020

06kellyjac commented Nov 17, 2020

jlesquembre commented Nov 18, 2020

06kellyjac commented Nov 18, 2020

jlesquembre commented Nov 18, 2020

06kellyjac commented Nov 18, 2020

marvin-mk2 bot commented Nov 21, 2020

jlesquembre commented Nov 23, 2020

SuperSandro2000 Nov 23, 2020

06kellyjac Nov 23, 2020 •

edited

kevincox Nov 24, 2020

kevincox Nov 24, 2020

SuperSandro2000 commented Nov 26, 2020

06kellyjac commented Nov 26, 2020

	description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container - k3d";
	description = "A helper to run k3s (Lightweight Kubernetes. 5 less than k8s) in a docker container";

kube3d: increase discoverability #99563

kube3d: increase discoverability #99563

Conversation

06kellyjac commented Oct 4, 2020 • edited

Motivation for this change

Things done

nixos-discourse commented Oct 22, 2020

06kellyjac commented Nov 17, 2020

marvin-mk2 bot commented Nov 17, 2020

06kellyjac commented Nov 17, 2020

jlesquembre commented Nov 18, 2020

06kellyjac commented Nov 18, 2020

jlesquembre commented Nov 18, 2020

06kellyjac commented Nov 18, 2020

marvin-mk2 bot commented Nov 21, 2020

jlesquembre commented Nov 23, 2020

SuperSandro2000 Nov 23, 2020

Choose a reason for hiding this comment

06kellyjac Nov 23, 2020 • edited

Choose a reason for hiding this comment

kevincox Nov 24, 2020

Choose a reason for hiding this comment

kevincox Nov 24, 2020

Choose a reason for hiding this comment

SuperSandro2000 commented Nov 26, 2020

06kellyjac commented Nov 26, 2020

06kellyjac commented Oct 4, 2020 •

edited

06kellyjac Nov 23, 2020 •

edited