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

nodePackages: init hsd, hs-airdrop, hs-client #98725

Merged
merged 4 commits into from Nov 17, 2020
Merged

nodePackages: init hsd, hs-airdrop, hs-client #98725

merged 4 commits into from Nov 17, 2020

Conversation

d-xo
Copy link
Contributor

@d-xo d-xo commented Sep 25, 2020

Motivation for this change

This adds a few tools related to the handshake decentralized naming system.

The following tools are added:

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.

@ofborg ofborg bot requested a review from midchildan September 25, 2020 08:52
midchildan
midchildan suggested changes Sep 25, 2020
View reviewed changes
Copy link
Member

@midchildan midchildan left a comment

Choose a reason for hiding this comment

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

The code looks good, but the commits should be separated for each new package you introduce to fit CONTRIBUTING.md. Since regenerating nodePackages would take a lot of time, you can just recreate the commits without changing the contents of this PR itself:

git reset --mixed HEAD~ # undos the commit without changing the working tree
git add pkgs/development/node-packages/default.nix
git add -p pkgs/development/node-packages/node-packages.json # add hsd
git commit -m "nodePackages.hsd: init at 2.2.0"
git add -p pkgs/development/node-packages/node-packages.json # add hs-airdrop
git commit -m "nodePackages.hs-airdrop: init at 0.9.0"
git add -p pkgs/development/node-packages/node-packages.json # add hs-client
git commit -m "nodePackages.hs-client: init at 0.0.9"
git add pkgs/development/node-packages/node-packages.nix
git commit -m "nodePackages: update"
git push -f

@d-xo
Copy link
Contributor Author

d-xo commented Sep 27, 2020

Updated commits and rebaed on master. Thanks for the help!!

@ofborg ofborg bot requested a review from midchildan September 27, 2020 22:22
midchildan
midchildan approved these changes Sep 28, 2020
View reviewed changes
Copy link
Member

@midchildan midchildan left a comment

Choose a reason for hiding this comment

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

LGTM

@d-xo
Copy link
Contributor Author

d-xo commented Nov 16, 2020

rebased on master, would be great to get this merged...

@ofborg ofborg bot requested a review from midchildan November 16, 2020 18:20
midchildan
midchildan approved these changes Nov 17, 2020
View reviewed changes
Copy link
Member

@midchildan midchildan left a comment

Choose a reason for hiding this comment

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

It sometimes helps to leave a comment here when your PR gets buried:

https://discourse.nixos.org/t/prs-already-reviewed/2617

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/286

@AndersonTorres
Copy link
Member

This is a huge diff! I am a bit afraid of it.

@midchildan
Copy link
Member

Unfortunately, this is unavoidable due to the way node packaging is done in nixpkgs. The source of the huge diff is node-packages.nix, which is generated automatically by this script here: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/node-packages/generate.sh

@AndersonTorres
Copy link
Member

Well, if nothing can be done...

@AndersonTorres AndersonTorres merged commit 224e1d5 into NixOS:master Nov 17, 2020
@d-xo d-xo deleted the handshake branch November 17, 2020 12:35
@dotlambda
Copy link
Member

dotlambda commented Aug 19, 2023 •
edited

@d-xo What's the purpose of having hs-client in nixpkgs? It's only a JS library.

@dotlambda dotlambda mentioned this pull request Aug 19, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@midchildan midchildan midchildan approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@d-xo @nixos-discourse @AndersonTorres @midchildan @dotlambda @ryantm