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

feat: add --hook to run commands when done thinking #366

Closed
wants to merge 1 commit into from

Conversation

cafkafk
Copy link

@cafkafk cafkafk commented Oct 11, 2023

This adds a --hook flag to the pr subcommand.

This is useful for e.g. running notify-send "done building", sending mail, or
any arbitrary thing you might wanna do when nixpkgs-review is done bulding.

It's been a long time since I wrote any python, and I have no grasp on this
codebase, so appologies if I made a mess!

@Artturin
Copy link
Collaborator

test fail

@@ -579,6 +584,7 @@ def review_local_revision(
build_graph=args.build_graph,
nixpkgs_config=nixpkgs_config,
extra_nixpkgs_config=args.extra_nixpkgs_config,
hook=args.hook,
Copy link
Owner

Choose a reason for hiding this comment

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

This Result class also needs this field. You can use mypy nixpkgs_review to test it for type errors like this.

Copy link
Author

Choose a reason for hiding this comment

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

When I run it I get:

mypy nixpkgs_review
Success: no issues found in 20 source files

Not sure what a result class is in this context

Copy link
Collaborator

Choose a reason for hiding this comment

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

When I run it I get:

mypy nixpkgs_review
Success: no issues found in 20 source files

Not sure what a result class is in this context

Rebase the branch, before #371 a --strict was needed

@cafkafk
Copy link
Author

cafkafk commented Oct 26, 2023

I'm not sure I understand python enough to fix this for now, might reopen later

@cafkafk cafkafk closed this Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants