Skip to content

cmd/api: Respect GH_REPO when substituting {owner}/{repo}#5063

Merged
mislav merged 1 commit intocli:trunkfrom
issyl0:repo-envvar-compatible-with-gh-api
Jan 20, 2022
Merged

cmd/api: Respect GH_REPO when substituting {owner}/{repo}#5063
mislav merged 1 commit intocli:trunkfrom
issyl0:repo-envvar-compatible-with-gh-api

Conversation

@issyl0
Copy link
Contributor

@issyl0 issyl0 commented Jan 19, 2022

  • Fixes GH_REPO compatibility with gh api #5061.
  • Previously, a user could set GH_REPO=TestOrg/repo and run gh api repos/{owner}/{repo}/releases expecting to see the releases from TestOrg/repo but instead see releases from the current git repo they're in.
  • Or, worse, if the user ran the command from outside of a git repo, they'd see a "not a git repo" error from git itself.
  • This was different to GH_REPO=TestOrg/repo gh release list which successfully lists TestOrg/repo's releases.

TODO and NOTES:

  • Test cases. Not sure how to do that tonight. For manual reproduction of the issue and testing my fix, I used a repo of mine that had releases, and one that did not (as you'll see from the before/after output below). Not necessary now we use OverrideBaseRepoFunc in the Cobra command definition.
  • I found an EnableRepoOverride function, but I couldn't work out how to use it with only GH_REPO. It seemed to relate heavily to the --repo flag, and telling users to specify --repo in gh api seemed weird.
  • This might be overly clunky in terms of code style and Go conventions and abstractions for this specific codebase - it's been a while. Comments, instruction and improvements welcome!

(Also worth noting I'm doing this right now entirely in a personal capacity for fun - it's an open source repo after all!)


Before:

$ cd repos/issyl0/not-a-git-repo
$ GH_REPO=issyl0/terraform-provider-improvmx gh api repos/{owner}/{repo}/releases
unable to expand placeholder in path: fatal: not a git repository (or any of the parent directories): .git
/opt/homebrew/bin/git: exit status 128
$ cd repos/issyl0/a-git-repo-with-no-releases
$ GH_REPO=issyl0/terraform-provider-improvmx gh api repos/{owner}/{repo}/releases
[]

After:

$ cd repos/issyl0/not-a-git-repo
$ GH_REPO=issyl0/terraform-provider-improvmx ../../cli/cli/bin/gh api repos/{owner}/{repo}/releases
[lots of JSON about GH_REPO's releases - success]
$ cd repos/issyl0/a-git-repo-with-no-releases
$ GH_REPO=issyl0/terraform-provider-improvmx ../../cli/cli/bin/gh api repos/{owner}/{repo}/releases
[lots of JSON about GH_REPO's releases - success]

- This fixes issue 5061.
- Previously, a user could set `GH_REPO=TestOrg/repo` and run `gh api
  repos/{owner}/{repo}/releases` expecting to see the releases from
  `TestOrg/repo` but instead see releases from the current git repo
  they're in.
- Or, worse, if the user ran the command from outside of a git repo,
  they'd see a "not a git repo" error from git itself.
- This was different to `GH_REPO=TestOrg/repo gh release list` which
  successfully lists `TestOrg/repo`'s releases.

----

Before:

```shell
$ cd repos/issyl0/not-a-git-repo
$ GH_REPO=issyl0/terraform-provider-improvmx gh api repos/{owner}/{repo}/releases
unable to expand placeholder in path: fatal: not a git repository (or any of the parent directories): .git
/opt/homebrew/bin/git: exit status 128
```

```shell
$ cd repos/issyl0/a-git-repo-with-no-releases
$ GH_REPO=issyl0/terraform-provider-improvmx gh api repos/{owner}/{repo}/releases
[]
```

After:

```shell
$ cd repos/issyl0/not-a-git-repo
$ GH_REPO=issyl0/terraform-provider-improvmx ../../cli/cli/bin/gh api repos/{owner}/{repo}/releases
[lots of JSON about GH_REPO's releases - success]
```

```shell
$ cd repos/issyl0/a-git-repo-with-no-releases
$ GH_REPO=issyl0/terraform-provider-improvmx ../../cli/cli/bin/gh api repos/{owner}/{repo}/releases
[lots of JSON about GH_REPO's releases - success]
```
@issyl0 issyl0 force-pushed the repo-envvar-compatible-with-gh-api branch from af49eeb to 8c2695b Compare January 19, 2022 23:17
@issyl0
Copy link
Contributor Author

issyl0 commented Jan 19, 2022

Thank you, @mislav, for the feedback - I pushed an improved version.

@issyl0 issyl0 marked this pull request as ready for review January 19, 2022 23:20
@issyl0 issyl0 requested a review from a team as a code owner January 19, 2022 23:20
@issyl0 issyl0 requested review from mislav and removed request for a team January 19, 2022 23:20
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Jan 19, 2022
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Thank you!!

@mislav mislav merged commit fac020e into cli:trunk Jan 20, 2022
@issyl0 issyl0 deleted the repo-envvar-compatible-with-gh-api branch January 20, 2022 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GH_REPO compatibility with gh api

3 participants