♻️ refactor: align lightweight issue views and command helpers#36
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41057731c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pager = TimelinePager(client) | ||
|
|
||
| meta = client.resolve_pull_request(selector=args.pr, repo=args.repo) | ||
| meta = _resolve_pr_meta(client=client, args=args) |
There was a problem hiding this comment.
pr view --repo working without an explicit --pr
cmd_pr_view() used to delegate directly to GitHubClient.resolve_pull_request(selector=args.pr, repo=args.repo), and that resolver still intentionally omits the selector when it is unset (src/gh_llm/github_api.py:707-711). That matches the GitHub CLI manual for gh pr view, which says “Without an argument, the pull request that belongs to the current branch is displayed.” After switching this call site to _resolve_pr_meta(), gh-llm pr view --repo <owner/repo> now fails with ``--pris required when--repo` is provided`, so users who pass `--repo` only to make repo selection explicit lose the current-branch PR workflow that worked before this refactor.
Useful? React with 👍 / 👎.
ShigureNyako
left a comment
ShigureNyako
left a comment
There was a problem hiding this comment.
独立审阅结论：APPROVE。
说明：当前登录账号就是该 PR 的作者，GitHub 不允许对自己的 PR 提交 APPROVE，因此这里改为以正式 COMMENT review 记录审阅结论。
审阅结论与依据如下：
- 功能设计：这次把
issue view的轻量路径补齐到与pr view一致；当--show不包含timeline时，不再无条件做 timeline bootstrap，边界更清晰，也符合轻量查看场景的预期。 - CLI 交互：
--body/--body-file以及--pr/--issue的解析与校验被收敛到共享 helper 后，命令表面行为保持不变，但重复逻辑明显减少，一致性更好。 - 测试覆盖：新增了
issue view轻量路径回归测试，以及 option helper 的默认值、--repo+selector 约束、空字符串body_file等边界用例。 - 本地验证：我额外跑了
uv run pytest -q、uv run ruff check .、uv run pyright，均通过。
一个非阻塞的小建议：README / skills/github-conversation/SKILL.md 里关于“non-timeline --show 走 metadata-only loading”的表述，后续可以再收紧一点，避免和 pr view --show checks / --show mergeability 这类仍会补抓状态信息的路径混淆。不过这不影响当前实现合并。
2 participants
Summary
Follow-up review for tracking issue #24 after the merged feature batch in #30, #31, #32, #33, #34, and #35.
This PR keeps the scope intentionally small: it tightens a few shared abstractions, removes one remaining PR/Issue inconsistency, and adds regression coverage around the cleaned-up paths.
Pure refactor
--body/--body-fileparser wiring--repo+ subject validation pathtests/test_command_options.pyBehavior-preserving cleanup
pr.pyandissue.py--showcombinations inREADME.mdandskills/github-conversation/SKILL.mdSmall behavior adjustment
issue viewwith the lightweight-loading behavior already added topr view: when--showdoes not includetimeline, the command now skips timeline bootstrap instead of always fetching first/last timeline pagesValidation
uv run pytest -quv run ruff check .uv run pyrightprettier --check README.md skills/github-conversation/SKILL.mduv run gh-llm issue view 24 --repo ShigureLab/gh-llm --show summary,actionsRisk
Low.
The main observable change is limited to non-timeline
issue viewcalls. The rest of the diff is helper consolidation plus regression coverage, so the primary risk is accidental argparse wiring drift, which is covered by the existing CLI tests and the new option-helper tests.Related Issues
--body-filesupport to PR review write actions #34, and ✨ feat: unify file-based inputs for comment edits and suggestions #35@SigureMo Please review this PR when you have time. Thanks!
Closes #24