Skip to content

refactor: break up ChecksNotifier.notify into focused methods [2/4]#695

Draft
thomasrockhu-codecov wants to merge 1 commit intorefactor/status-notifier-breakupfrom
refactor/checks-notifier-breakup
Draft

refactor: break up ChecksNotifier.notify into focused methods [2/4]#695
thomasrockhu-codecov wants to merge 1 commit intorefactor/status-notifier-breakupfrom
refactor/checks-notifier-breakup

Conversation

@thomasrockhu-codecov
Copy link
Contributor

Summary

Extracts ChecksNotifier.notify() (176 lines) into focused methods under 50 lines each. Pure refactoring, no behavior changes. Removes unused nullcontext and get_commit_url imports.

Method Responsibility
notify() Orchestrates: PR validation → preconditions → status check → build → send
_validate_pull_request() Validates PR exists, is in provider, is open
_check_preexisting_status() Checks if commit status already exists
_build_payload_with_behavior() Overrides parent with checks-specific output logic

Inherits _check_notify_preconditions() and _set_payload_url() from StatusNotifier (PR 1/4).

Test plan

  • test_checks.py: 57/57 passed
  • ruff check + ruff format: passing

Stack: This is PR 2/4. Depends on PR 1/4 (#694).

Made with Cursor

@sentry
Copy link

sentry bot commented Feb 7, 2026

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.35%. Comparing base (4935682) to head (814ca05).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ker/services/notification/notifiers/checks/base.py 95.23% 2 Missing ⚠️
Additional details and impacted files
@@                        Coverage Diff                        @@
##           refactor/status-notifier-breakup     #695   +/-   ##
=================================================================
  Coverage                             92.34%   92.35%           
=================================================================
  Files                                  1302     1302           
  Lines                                 47818    47829   +11     
  Branches                               1619     1619           
=================================================================
+ Hits                                  44159    44171   +12     
+ Misses                                 3350     3349    -1     
  Partials                                309      309           
Flag Coverage Δ
workerintegration 58.57% <7.14%> (-0.03%) ⬇️
workerunit 90.39% <95.23%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-notifications
Copy link

codecov-notifications bot commented Feb 7, 2026

Codecov Report

❌ Patch coverage is 95.23810% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ker/services/notification/notifiers/checks/base.py 95.23% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Extract ChecksNotifier.notify() (176 lines) into focused methods
under 50 lines each.

- _validate_pull_request(): validates PR exists, is in provider, is open
- _check_preexisting_status(): checks if commit status already exists
- _build_payload_with_behavior(): overrides parent with checks-specific logic
- Removed unused nullcontext and get_commit_url imports

Co-authored-by: Cursor <cursoragent@cursor.com>
@thomasrockhu-codecov thomasrockhu-codecov force-pushed the refactor/checks-notifier-breakup branch from 814ca05 to 35c9ff1 Compare February 8, 2026 10:16
@thomasrockhu-codecov thomasrockhu-codecov force-pushed the refactor/status-notifier-breakup branch from 4935682 to 268f901 Compare February 8, 2026 10:16
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.

1 participant