Skip to content

Centralize GH CLI timeout constant across maintenance scripts (#936)#1178

Open
yanyishuai wants to merge 1 commit into
ramimbo:mainfrom
yanyishuai:fix/issue-936-gh-cli-constants
Open

Centralize GH CLI timeout constant across maintenance scripts (#936)#1178
yanyishuai wants to merge 1 commit into
ramimbo:mainfrom
yanyishuai:fix/issue-936-gh-cli-constants

Conversation

@yanyishuai

Copy link
Copy Markdown

Summary

Centralizes the duplicated GH_TIMEOUT_SECONDS = 30 literal used across seven read-only GitHub CLI maintenance scripts into scripts/gh_cli_constants.py.

Why

Several maintenance scripts already share safety-cap and JSON-shape helpers; the timeout constant was still duplicated by hand. A single module reduces drift when the timeout policy changes.

Tests

python -m pytest tests/test_gh_cli_constants.py -q

Existing script behavior is unchanged aside from the shared import.

Wallet

Do4v7foHJvRJLpRRoGaVPWX6DDEjX3yTK7J91gpwUQpE

Closes #936

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@yanyishuai, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 27 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 69c8e6c5-df82-4455-81c5-ead21a6d8e55

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc87d2 and 53070c1.

📒 Files selected for processing (9)
  • scripts/check_bounty_issue_states.py
  • scripts/check_live_bounty_closing_refs.py
  • scripts/claim_inventory.py
  • scripts/gh_cli_constants.py
  • scripts/pr_queue_health.py
  • scripts/proposed_work_triage.py
  • scripts/review_bounty_candidates.py
  • scripts/submission_quality_gate.py
  • tests/test_gh_cli_constants.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qingfeng312 qingfeng312 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CI is failing during pytest collection because the new timeout helper module is not present where the changed scripts import it.

tests/test_submission_quality_gate.py imports scripts.submission_quality_gate, which now imports from scripts.public_payment_language, but that module is missing in this PR's checked-out tree. The workflow stops with:

ModuleNotFoundError: No module named 'scripts.public_payment_language'

Please include the missing module, or update the imports so the test suite can collect successfully.

Evidence: GitHub Actions run 28344111604, job 83964056348, collection interrupted with 1 error.

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 Rebuilt this branch from ramimbo/main so it only centralizes GH_TIMEOUT_SECONDS — no accidental #1107 payment-language imports.

  • Added scripts/gh_cli_constants.py
  • Migrated the seven maintenance scripts + tests/test_gh_cli_constants.py
  • Fixed review_bounty_candidates.py sys.path bootstrap for subprocess --help
  • Ruff import order clean

Latest head: fdba69b11a67. CI: pass. Please re-review.

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

1 similar comment
@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

@qingfeng312 qingfeng312 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Follow-up on current head fdba69b11a67eaef982c56e157a4cce444af2fd4.

The earlier missing-module collection blocker has been resolved. I rechecked the new scripts/gh_cli_constants.py helper and the maintenance scripts migrated to use the shared GH_TIMEOUT_SECONDS value.

The branch is now self-contained, limits itself to the timeout-constant refactor plus the subprocess help bootstrap cleanup, and adds focused coverage for the constant.

Validation checked: GitHub CI Quality, readiness, docs, and image checks passed on run 28350097261; CodeRabbit status is success on this head. I did not find a remaining blocker for the scoped #936 change.

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

4 similar comments
@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (fdba69b11a67) for #936. Could you take another look when you have a moment?

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 — proactive CRLF cleanup on this branch.

Normalized LF line endings (no functional changes) in:

  • scripts/check_bounty_issue_states.py
  • scripts/check_live_bounty_closing_refs.py
  • scripts/claim_inventory.py
  • scripts/pr_queue_health.py
  • scripts/proposed_work_triage.py
  • scripts/review_bounty_candidates.py
  • scripts/submission_quality_gate.py

Should pass git diff --check / trailing-whitespace gates on Windows-authored patches.

@yanyishuai yanyishuai force-pushed the fix/issue-936-gh-cli-constants branch from fdba69b to 53070c1 Compare July 3, 2026 02:04

@piaigmt piaigmt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE — head 53070c1d73eb1ef91c67a2fc4356184d825e389c

Clean, behavior-preserving DRY refactor of the duplicated GH_TIMEOUT_SECONDS = 30 constant into a single shared module scripts/gh_cli_constants.py, plus a focused regression test.

Local verification (Py 3.12 venv, at head 53070c1d):

  • No stray duplicates left: grep -rn "GH_TIMEOUT_SECONDS = 30" scripts/ | grep -v gh_cli_constants.py → empty. The canonical 30 definition now lives only in gh_cli_constants.py.
  • All 7 consumers wired to the shared module: grep -rln "from scripts.gh_cli_constants import GH_TIMEOUT_SECONDS" scripts/ → 7 files (check_bounty_issue_states, check_live_bounty_closing_refs, claim_inventory, pr_queue_health, proposed_work_triage, review_bounty_candidates, submission_quality_gate).
  • No ImportError introduced: python -c "import scripts.<m>" for all 7 consumers → all OK. The review_bounty_candidates.py sys.path bootstrap for __package__ in {None, ""} correctly handles direct-script invocation.
  • Value preserved: GH_TIMEOUT_SECONDS == 30 (int) — pure consolidation, no timeout change.
  • New test passes: pytest tests/test_gh_cli_constants.py -q1 passed.
  • ruff clean: ruff check on all 9 touched files → All checks passed; ruff format --check → already formatted.
  • CI green: CodeRabbit pass; "Quality, readiness, docs, and image checks" pass.

Non-tautological check: the test asserts both isinstance(GH_TIMEOUT_SECONDS, int) and > 0, which guards against a future accidental string/negative override of the shared constant.

No self-review conflict (author yanyishuai, reviewer piaigmt). Mergeable, no blockers.

@yanyishuai

Copy link
Copy Markdown
Author

@qingfeng312 CI is green on the latest head (53070c1d73eb) for #936. Could you take another look when you have a moment?

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.

MRWK bounty: 100 MRWK - code cleanup and maintainability improvements, round 2

3 participants