fix: support crit pull across gh CLI versions#299
fix: support crit pull across gh CLI versions#299tomasz-tomczyk merged 2 commits intotomasz-tomczyk:mainfrom
Conversation
GitHub CLI added `gh api --slurp` in v2.48.0, but older installs still need to fetch paginated PR comments successfully. Detect whether the local `gh` supports `--slurp` and use the native `--paginate --slurp` path when available. For older gh versions, fall back to `--paginate --jq .[]` and decode the streamed JSON objects. This keeps `crit pull` working on both pre-2.48.0 and newer gh releases, with a TODO to remove the compatibility branch once Crit requires `gh v2.48.0+`.
There was a problem hiding this comment.
Pull request overview
This PR keeps crit pull compatible across GitHub CLI versions by detecting whether the locally installed gh supports gh api --slurp (added in v2.48.0) and selecting the appropriate pagination/decoding strategy.
Changes:
- Add
gh versionparsing helpers to detect--slurpsupport based on installed GitHub CLI version. - Split PR comment fetching into
--paginate --slurp(newer gh) vs--paginate --jq .[]with streamed-object decoding (older gh). - Add unit tests covering
ghversion output parsing for--slurpsupport.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
github.go |
Adds version detection and a fallback pagination+decode path so PR comments can be fetched on pre-2.48.0 gh installs. |
github_test.go |
Adds tests validating version-output parsing for determining --slurp availability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you for the contribution! I checked and 2.48.0 has been out for 2 years now so I feel it's fair to say one should just upgrade and we can make it explicit that we require that version or above. Are there any compatibility issues you found that prevent upgrading If there's no issues per se and you just got thrown off by it not working with the older version, I'd be happy to accept a PR that instead makes it clear to the user/throws an error/somehow makes it obvious it's required? |
|
The main issue is that on Ubuntu 24.04 LTS, installing So this is not just about me personally not upgrading - users doing a normal I agree that requiring |
Use errors.Is(err, io.EOF) instead of direct comparison, which is idiomatic Go since 1.13 and defensive against future error wrapping. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Ok, that is a compatibility issue then! Thanks for clarifying, will merge 💜 💙 💚 |
GitHub CLI added
gh api --slurpin v2.48.0, but older installsstill need to fetch paginated PR comments successfully.
Detect whether the local
ghsupports--slurpand use the native--paginate --slurppath when available. For older gh versions, fallback to
--paginate --jq .[]and decode the streamed JSON objects.This keeps
crit pullworking on both pre-2.48.0 and newer gh releases,with a TODO to remove the compatibility branch once Crit requires
gh v2.48.0+.