fix: accept colon-linked bounty issue refs#557
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR relaxes the ChangesLinked-issue regex relaxation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Reviewed PR #557 at Evidence checked:
No functional blocker found in isolation for the new Merge-selection note: this appears to be superseded by the already-open PR #540. Both PRs target the same colon-linked bounty reference behavior for #406 and both modify the same The difference is coverage depth:
Recommendation: avoid merging both. Either proceed with #540 or explicitly choose one implementation/test set and close the duplicate. If #540 lands first, #557 should be closed as a duplicate/superseded PR. |
Baijack-star
left a comment
There was a problem hiding this comment.
Reviewed current head c54f7c0d46dde5a9e38d57dd4175c60325693c05 as a non-author, focused on the accepted-label webhook colon-reference change.\n\nI would not merge or pay this PR as a distinct #406 fix in its current form because it duplicates the already-open PR #540 scope. Both PRs change the same LINKED_ISSUE_RE colon separator behavior in app/webhooks/github.py and add the same accepted-label payout regression surface in tests/test_webhooks.py. Comparing #557 directly against PR #540 shows no new product behavior: #557 only changes the regex spelling from #540's (?:\s+|\s*:\s*) to \s*:?\s+, and swaps the test fixture from Bounty: #3 to Refs: #406.\n\nThe current #557 regression is also weaker than #540's already-reviewed coverage: it verifies a single paid result, but drops #540's duplicate-delivery replay assertion and webhook event processed_status == paid check. That makes this a superseded duplicate rather than a useful additional fix.\n\nValidation I ran on #557:\n- PYTHONPATH=. /Users/apple/Documents/paid-demand-goal-reset/mergework-review/.venv/bin/python -m pytest tests/test_webhooks.py -q -> 19 passed\n- PYTHONPATH=. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /Users/apple/Documents/paid-demand-goal-reset/mergework-review/.venv/bin/python -m pytest -q -> 415 passed\n- PYTHONPATH=. /Users/apple/Documents/paid-demand-goal-reset/mergework-review/.venv/bin/python -m ruff check app/webhooks/github.py tests/test_webhooks.py -> passed\n- PYTHONPATH=. /Users/apple/Documents/paid-demand-goal-reset/mergework-review/.venv/bin/python -m ruff format --check app/webhooks/github.py tests/test_webhooks.py -> 2 files already formatted\n- PYTHONPATH=. /Users/apple/Documents/paid-demand-goal-reset/mergework-review/.venv/bin/python -m mypy app/webhooks/github.py -> success\n- git diff --check origin/main...HEAD -> clean\n\nSuggested path: close this PR as superseded by #540, or re-scope it only if it adds a genuinely distinct webhook parser boundary not already covered by #540/#539/#555.
Summary
Refs: #406Why
GitHub contributors commonly write references as
Refs: #123orFixes: #123. The current payout parser only acceptsRefs #123, so a maintainer could applymrwk:acceptedto valid work and still getmissing_issueif the PR body used the colon form.Tests
./.venv/bin/python -m ruff format --check app/webhooks/github.py tests/test_webhooks.py./.venv/bin/python -m ruff check app/webhooks/github.py tests/test_webhooks.py./.venv/bin/python -m pytest tests/test_webhooks.py -q./.venv/bin/python -m pytest -qRefs #406
Summary by CodeRabbit
Bug Fixes
Refs: 123,Closes: 456), improving compatibility with common linking conventions.Tests