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

doc: add instructions to remove a package #116475

Merged
merged 1 commit into from
Apr 22, 2021

Conversation

davidak
Copy link
Member

@davidak davidak commented Mar 15, 2021

Motivation for this change

It was not documented and i had to reverse engineer it.

It don't really fit in that place and the style is different than the rest. I wish every task would be explained that detailed, so everyone can do it.

Preview

https://github.com/NixOS/nixpkgs/pull/116475/files?short_path=ba88122#diff-ba881223dddc816723e79c2ee074df7b18a08b6520273c22665143b700f3f52d

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 added 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Mar 15, 2021
@SuperSandro2000
Copy link
Member

FYI I can just press that button for a preview.

image

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Great idea!

I think grammatical correct sentences need to start with a capital letter.

doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
@nrdxp nrdxp added this to the 21.05 milestone Mar 24, 2021
@davidak davidak force-pushed the doc-remove-packages branch from acc4de7 to a6e1655 Compare April 3, 2021 04:05
@davidak davidak marked this pull request as ready for review April 3, 2021 04:09
@davidak
Copy link
Member Author

davidak commented Apr 3, 2021

I finally found the time to update it. Feel free to give feedback!

@davidak
Copy link
Member Author

davidak commented Apr 3, 2021

@FRidh where is the right place to add python aliases?

@FRidh
Copy link
Member

FRidh commented Apr 3, 2021

Package sets typically do not have aliases.

doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
@davidak davidak force-pushed the doc-remove-packages branch from a6e1655 to 7ebb9e0 Compare April 3, 2021 13:34
@SuperSandro2000
Copy link
Member

@FRidh where is the right place to add python aliases?

Not merged yet.

@davidak
Copy link
Member Author

davidak commented Apr 13, 2021

Can we get this merged?

@SuperSandro2000 @FRidh

@davidak
Copy link
Member Author

davidak commented Apr 17, 2021

Can you add your review again?

@davidak davidak mentioned this pull request Apr 17, 2021
2 tasks
Copy link
Member

@siraben siraben left a comment

Choose a reason for hiding this comment

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

Overall structure looks OK. Suggested grammatical/phrasing fixes. Not sure if the list numbering staying at 1. was intentional or not.

EDIT: list numbering is fine on export

doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
doc/contributing/submitting-changes.chapter.md Outdated Show resolved Hide resolved
Co-authored-by: Sandro <[email protected]>
Co-authored-by: Ben Siraphob <[email protected]>
@davidak davidak force-pushed the doc-remove-packages branch from b336ef3 to c0b5d5b Compare April 20, 2021 20:30
@davidak
Copy link
Member Author

davidak commented Apr 20, 2021

@siraben thanks. i accepted the suggestions except one. take a look if you agree that this makes sense

i also squashed, so it could get merged now

@samueldr samueldr merged commit 5ce39b7 into NixOS:master Apr 22, 2021
@davidak davidak deleted the doc-remove-packages branch April 22, 2021 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants