Skip to content

buildGoPackage: deps.json -> deps.nix in NIXON#18487

Closed
kamilchm wants to merge 4 commits intoNixOS:masterfrom
kamilchm:buildGoPackage
Closed

buildGoPackage: deps.json -> deps.nix in NIXON#18487
kamilchm wants to merge 4 commits intoNixOS:masterfrom
kamilchm:buildGoPackage

Conversation

@kamilchm
Copy link
Member

Motivation for this change

#17254 (comment)

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@kamilchm, thanks for your PR! By analyzing the annotation information on this pull request, we identified @zimbatm, @offlinehacker and @rushmorem to be potential reviewers

@rushmorem
Copy link
Contributor

The packages build fine but it seems like you forgot to update the docs. May I also suggest that we take this opportunity to mention the removal of goPackages in nixos/doc/manual/release-notes/rl-1609.xml.

@kamilchm
Copy link
Member Author

kamilchm commented Sep 10, 2016

Docs updated.

@rushmorem
Copy link
Contributor

Had opened an issue for the release notes (#18496) but I see you have since added them. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Is import necessary here? I don't see it included in the packages you converted from deps.json.

Copy link
Member

Choose a reason for hiding this comment

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

you and me know what NIXON means but not the reader.

The dependency data structure is described below.

@zimbatm
Copy link
Member

zimbatm commented Sep 11, 2016

Looking good! Let me know if you want to change the NIXON references, otherwise I'll just merge it and add it to the 16.09 branch.

@kamilchm
Copy link
Member Author

@zimbatm I replaced NIXON with nix as you suggested.

@kamilchm
Copy link
Member Author

Telegraf needs to be converted #18437

@zimbatm
Copy link
Member

zimbatm commented Sep 15, 2016

Let me merge this and then update telegraph

@zimbatm
Copy link
Member

zimbatm commented Sep 15, 2016

Pushed as 914e0e5 on master. I squashed the commits to make it easier to cherry-pick onto the 16.09 branch.

@zimbatm
Copy link
Member

zimbatm commented Sep 15, 2016

Pushed as 9ab3dc2 on the release-16.09 branch. etcd had to be fixed.

@zimbatm
Copy link
Member

zimbatm commented Sep 15, 2016

I think everything's good now! Thanks a lot @kamilchm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants