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

nix-doc: init at 0.3.3 #95161

Merged
merged 2 commits into from Aug 14, 2020
Merged

nix-doc: init at 0.3.3 #95161

merged 2 commits into from Aug 14, 2020

Conversation

lf-
Copy link
Member

@lf- lf- commented Aug 11, 2020

Motivation for this change

It was requested by @gvolpe :)

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.

re: the ??? above, nixpkgs-review did not find anything when I ran it post-commit, and nix path-info -S hit the intentionally failing pseudo-package, so I'm not sure how I'm supposed to do that piece.

@lf-
Copy link
Member Author

lf- commented Aug 11, 2020

Apologies about the sneaky edits, there was a pretty nasty bug I found and squished while doing final validation on the package.

@lf- lf- force-pushed the patch-1 branch 3 times, most recently from 2aec9e1 to 9fbf770 Compare August 11, 2020 12:18
@lf- lf- changed the title nix-doc: init at 0.3.2 nix-doc: init at 0.3.3 Aug 11, 2020
@lf-
Copy link
Member Author

lf- commented Aug 11, 2020

OK I am done smuggling in changes. It builds and runs now with no issues, and contains two fewer bugs than when I started :)

@gvolpe
Copy link
Member

gvolpe commented Aug 11, 2020

@lf- you will be asked to create a separate commit to add yourself as a maintainer as maintainers: add lf-. Ideally, this commit should come first :)

@gvolpe
Copy link
Member

gvolpe commented Aug 11, 2020

You can also ignore that ??? as it doesn't apply to this new package.

Copy link
Member

@IvarWithoutBones IvarWithoutBones left a comment

Choose a reason for hiding this comment

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

Left a few suggestions, other than that LGTM :)

maintainers/maintainer-list.nix Show resolved Hide resolved
pkgs/tools/package-management/nix-doc/default.nix Outdated Show resolved Hide resolved
pkgs/tools/package-management/nix-doc/default.nix Outdated Show resolved Hide resolved
pkgs/tools/package-management/nix-doc/default.nix Outdated Show resolved Hide resolved
pkgs/tools/package-management/nix-doc/default.nix Outdated Show resolved Hide resolved
@lf- lf- force-pushed the patch-1 branch 2 times, most recently from 50502e9 to 0220ace Compare August 12, 2020 01:23
@lf-
Copy link
Member Author

lf- commented Aug 12, 2020

Thanks for the review @IvarWithoutBones. The requested changes have been made.

@lf-
Copy link
Member Author

lf- commented Aug 12, 2020

@IvarWithoutBones changed :)

@Ma27 Ma27 merged commit d22cd37 into NixOS:master Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants