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.

Sign up for GitHub

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

trashy: Generate and install man page #260782

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

trashy: Generate and install man page #260782

wants to merge 1 commit into from

Conversation

stelcodes
Copy link
Contributor

@stelcodes stelcodes commented Oct 12, 2023 •
edited
Loading

Description of changes

Adds a man page for the trash command from the trashy package. Both man trash and alias man trashy are installed.

Since this is fairly insignificant I did not add a release note.

Tested with nix shell --sandbox --file ./default.nix trashy on a local nixpkgs branch with the added changes. Running man trash and man trashy inside the shell successfully shows the trashy man page.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Oct 13, 2023
@h7x4
Copy link
Member

h7x4 commented Oct 16, 2023 •
edited
Loading

It's probably not a big deal, but there seems to be a clash with https://github.com/NixOS/nixpkgs/blob/nixos-23.05/pkgs/os-specific/darwin/trash/default.nix#L23-L24. Not sure if something should be done about it

On second thought, there might actually not be a clash atm, since this only runs on linux and the other package only runs on darwin.

h7x4
h7x4 approved these changes Oct 16, 2023
View reviewed changes
Copy link
Member

@h7x4 h7x4 left a comment

Choose a reason for hiding this comment

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

Can confirm man trash and man trashy works fine.

Result of nixpkgs-review pr 260782 run on x86_64-linux 1

1 package built:
  • trashy

@h7x4 h7x4 added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Oct 16, 2023
@stelcodes stelcodes requested review from oberblastmeister and removed request for oberblastmeister October 17, 2023 16:27
@stelcodes stelcodes mentioned this pull request Oct 22, 2023
Closed
pbsds
pbsds reviewed Nov 27, 2023
View reviewed changes
pkgs/tools/misc/trashy/default.nix
Comment on lines +21 to +23
$out/bin/trash manpage > trash.1
cp trash.1 trashy.1
installManPage trash.1 trashy.1
Copy link
Member

Choose a reason for hiding this comment

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

Considering oberblastmeister/trashy#106 is merged, will this work?

Suggested change
$out/bin/trash manpage > trash.1
cp trash.1 trashy.1
installManPage trash.1 trashy.1
$out/bin/trash manpage > trashy.1
installManPage trashy.1

Copy link
Member

Choose a reason for hiding this comment

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

You may have to bump to 2.0.0-unstable-2023-XX-YY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! I meant to comment on this PR about that trashy PR getting merged. Maybe we should just wait until trashy gets a new release? Is it normal to do an unstable version bump for a package that has a release cadence?

Copy link
Member

Choose a reason for hiding this comment

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

It's discouraged, and cherry picking a backward incompatible change as a patch without indicating so in the version is also disingenuous.

Copy link
Member

Choose a reason for hiding this comment

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

If the release will take too long, then consider this in the meanwhile

Suggested change
$out/bin/trash manpage > trash.1
cp trash.1 trashy.1
installManPage trash.1 trashy.1
$out/bin/trash manpage > trash.1
installManPage trash.1

pbsds
pbsds reviewed Nov 27, 2023
View reviewed changes
Copy link
Member

@pbsds pbsds left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 260782 run on x86_64-linux 1

1 package built:
  • trashy

pkgs/tools/misc/trashy/default.nix
Comment on lines +21 to +23
$out/bin/trash manpage > trash.1
cp trash.1 trashy.1
installManPage trash.1 trashy.1
Copy link
Member

Choose a reason for hiding this comment

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

You may have to bump to 2.0.0-unstable-2023-XX-YY

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Aleksanaa
Aleksanaa reviewed Jul 10, 2024
View reviewed changes
pkgs/tools/misc/trashy/default.nix
@@ -17,7 +17,10 @@ rustPlatform.buildRustPackage rec {
installShellCompletion --cmd trash \
Copy link
Member

Choose a reason for hiding this comment

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

See #308283

@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Jul 10, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aleksanaa Aleksanaa Aleksanaa left review comments

@pbsds pbsds pbsds left review comments

@h7x4 h7x4 h7x4 approved these changes

@oberblastmeister oberblastmeister Awaiting requested review from oberblastmeister

Assignees
No one assigned
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stelcodes @h7x4 @pbsds @Aleksanaa @wegank