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

birdtray: fix PATH handling, move to by-name #327363

Closed
wants to merge 2 commits into from

Conversation

eclairevoyant
Copy link
Contributor

@eclairevoyant eclairevoyant commented Jul 15, 2024

Description of changes

  • birdtray: move to by-name
  • birdtray: fix PATH handling

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

pkgs/by-name/bi/birdtray/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/bi/birdtray/package.nix Show resolved Hide resolved
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

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

I'll let someone else validate it.

I don't agree so much on touching files and not improving them and postpone that in another PR.

Hope you understand that this is not against you @eclairevoyant personally !

@eclairevoyant
Copy link
Contributor Author

eclairevoyant commented Jul 15, 2024

Sure, I get it. Re: pname use, I just don't like re-explaining / re-justifying the same arguments in multiple PRs to everyone who might disagree, and I'd rather have all the discussion about pname use in one place.

As for reformatting, I don't have a strong opinion either way, but folks such as @SuperSandro2000 apparently do. In any case I consider it busy work to have to keep adding entries to the .git-blame-ignore-revs, and have to keep updating it if I rebase the commits, etc. Unless we are reformatting an entire package set or something the effort doesn't seem worth it to me. It's also not enforced outside of certain paths that are already tracked in the CI (which would result in a CI fail).

I moved to by-name since that's not really automatable and (save for one person) I've basically never seen an objection to it. It also makes future changes easier.

@eclairevoyant
Copy link
Contributor Author

Closing as per #291745 (comment), this may still be the wrong approach.

@eclairevoyant eclairevoyant deleted the birdtray-path-fix branch July 15, 2024 14:48
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.

Birdtray hard codes a default path
3 participants