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
gsctl: init at 0.15.4 #63960
gsctl: init at 0.15.4 #63960
Conversation
@jagajaga @domenkozar mind having a look? |
@GrahamcOfBorg build |
|
||
src = fetchFromGitHub { | ||
owner = "giantswarm"; | ||
repo = "gsctl"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use pname
instead of name and use it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Look better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thank you!
Anything I can do to help with getting this merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @JosephSalisbury.
Typically, adding yourself to the maintainers list should be done in a separate commit like
maintainers: add joesalisbury
I've left some minor comments, but I can it does build and execute for me locally.
Hey @worldofpeace, that should be all the comments addressed. Mind taking another pass? |
@JosephSalisbury Why'd you drop adding yourself to the maintainers list? Hope I didn't confuse you. |
I possibly misinterpreted you - I was under the impression you wanted me to add it separately. I can add it back here, or follow up with it. What would you recommend? |
Your first commit in this PR should add yourself to
and the second commit should be the package init and you should be listed as a maintainer in this package. |
@worldofpeace hi! does that look better to you? |
Yep 👍 Not sure why the bot added |
There was one before I rebased, probably just a bit confused |
Thanks for contributing @JosephSalisbury. |
Thanks for helping! |
Motivation for this change
We have a few folks at Giant Swarm using nix, so I reckoned I’d add it here :D
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)