Skip to content

nox-review: fix wip click decorator - #80

Open
lsix wants to merge 1 commit into
madjar:masterfrom
lsix:fix-wip-command-name
Open

nox-review: fix wip click decorator#80
lsix wants to merge 1 commit into
madjar:masterfrom
lsix:fix-wip-command-name

Conversation

@lsix

@lsix lsix commented Sep 29, 2017

Copy link
Copy Markdown

Since f50bebb, the function the
@cli.command decorator sees is the one generated with
@setup_nixpkgs_config, i.e. _ (c.f. nox/review.py:66). Therefore it
cannot properly guess figure the wip function name.

This explains the behaviour observed in
NixOS/nixpkgs#29684:

$ nox-review --help
Usage: nox-review [OPTIONS] COMMAND [ARGS]...

  Review a change by building the touched commits

Options:
  -k, --keep-going  Keep going in case of failed builds
  --dry-run         Don't actually build packages, just print the
commands
                    that would have been run
  --help            Show this message and exit.

Commands:
  _   difference between working tree and a commit
  pr  changes in a pull request

This commit basically does what is done in nixpkgs to fix the behaviour
change:
NixOS/nixpkgs@5f0a7cd

Since f50bebb, the function the
`@cli.command` decorator sees is the one generated with
`@setup_nixpkgs_config`, i.e. `_` (c.f. nox/review.py:66). Therefore it
cannot properly guess figure the `wip` function name.

This explains the behaviour observed in
NixOS/nixpkgs#29684:

```
$ nox-review --help
Usage: nox-review [OPTIONS] COMMAND [ARGS]...

  Review a change by building the touched commits

Options:
  -k, --keep-going  Keep going in case of failed builds
  --dry-run         Don't actually build packages, just print the
commands
                    that would have been run
  --help            Show this message and exit.

Commands:
  _   difference between working tree and a commit
  pr  changes in a pull request
```

This commit basically does what is done in nixpkgs to fix the behaviour
change:
NixOS/nixpkgs@5f0a7cd
@bobvanderlinden

Copy link
Copy Markdown

I ran into this problem on nixpkgs. The documented command nix-shell -p nox --run "nox-review wip" currently shows an error:

Usage: nox-review [OPTIONS] COMMAND [ARGS]...

Error: No such command "wip".

With the current state of nox only nox-review _ will work. This PR is very much appreciated and it would be great to get it merged and into nixpkgs.

@lsix

lsix commented Oct 2, 2017

Copy link
Copy Markdown
Author

@bobvanderlinden there is already a fix in nixpkgs (NixOS/nixpkgs@5f0a7cd). I just ported it to stable so as soon as the channel has been updated by hydra it will be fixed in release-17.09 as far as the users are concerned.

@bobvanderlinden

Copy link
Copy Markdown

Ah, great. Thank you!

@madjar

madjar commented Oct 26, 2017

Copy link
Copy Markdown
Owner

Thanks for that!

I can't merge it right now, because that would break the CI (the default.nix depends on nixpkgs, so it tries to apply that patch). I'll merge this just before I make the next release.

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.

4 participants