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

Add zsh zhooks plugin #296562

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fidgetingbits
Copy link
Contributor

Description of changes

Add myself to maintainers list and add a new simple zsh plugin as my first package.

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@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-ready-for-review/3032/3645

pkgs/shells/zsh/zhooks/default.nix Outdated Show resolved Hide resolved
pkgs/shells/zsh/zhooks/default.nix Outdated Show resolved Hide resolved
pkgs/shells/zsh/zhooks/default.nix Outdated Show resolved Hide resolved
pkgs/shells/zsh/zhooks/default.nix Outdated Show resolved Hide resolved
pkgs/shells/zsh/zhooks/default.nix Outdated Show resolved Hide resolved
@fidgetingbits
Copy link
Contributor Author

fidgetingbits commented Mar 18, 2024

Thanks! All of the suggestions have been fixed.

One thing I remembered is that things should go into pkgs/by-name/ now so I also moved it and renamed it zsh-zhooks to match the naming of most other zsh plugins. Let me know if there are any concerns with that.

I also just remembered there is a push to not use with anymore, so also tweaked that.

@fidgetingbits
Copy link
Contributor Author

@SuperSandro2000 can you take another review pass if you find the time?

@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-ready-for-review/3032/3698

@DontEatOreo
Copy link
Member

Your commit is breaking Commit Conventions. Please rebase.

In your case reword 215bad7 to zhook: init at 0-unstable-10-31-2021 and squash 883e162 5cb02eb into 215bad7

pkgs/by-name/zs/zsh-zhooks/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/zs/zsh-zhooks/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/zs/zsh-zhooks/package.nix Outdated Show resolved Hide resolved
@DontEatOreo
Copy link
Member

Please rename your PR title to zhook: init at 0-unstable-10-31-2021 as well

@fidgetingbits
Copy link
Contributor Author

Thanks @DontEatOreo. Just noted the one difference between our nixfmt-rfc-style formatting, but otherwise should all be addressed.

@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/1630

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

5 participants