-
Notifications
You must be signed in to change notification settings - Fork 94
Clarify Slack messaging for low code coverage #27586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -711,13 +711,17 @@ jobs: | |||||||||||||||||||||||||||||||||||
| "type": "header", | ||||||||||||||||||||||||||||||||||||
| "text": { | ||||||||||||||||||||||||||||||||||||
| "type": "plain_text", | ||||||||||||||||||||||||||||||||||||
| "text": ":warning: Warning: Vets-API Coverage Below Target", | ||||||||||||||||||||||||||||||||||||
| "text": ":warning: Warning: Vets-API Coverage Below 93%", | ||||||||||||||||||||||||||||||||||||
| "emoji": true | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||
| "type": "section", | ||||||||||||||||||||||||||||||||||||
| "fields": [ | ||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||
| "type": "mrkdwn", | ||||||||||||||||||||||||||||||||||||
| "text": "_If coverage dips below 90%, CI, including deployments, will be blocked._" | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||
| "type": "mrkdwn", | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
720
to
726
|
||||||||||||||||||||||||||||||||||||
| "fields": [ | |
| { | |
| "type": "mrkdwn", | |
| "text": "_If coverage dips below 90%, CI, including deployments, will be blocked._" | |
| }, | |
| { | |
| "type": "mrkdwn", | |
| "text": { | |
| "type": "mrkdwn", | |
| "text": "_If coverage dips below 90%, CI, including deployments, will be blocked._" | |
| } | |
| }, | |
| { | |
| "type": "section", | |
| "fields": [ | |
| { | |
| "type": "mrkdwn", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning header now highlights the 93% threshold, but the block still includes a field labeled ":dart: Required Minimum: 90%" (deployment block threshold). That combination can read as internally inconsistent (is the requirement 93% or 90%?). Consider renaming the 90% field to something explicit like “Deployment block threshold” / “Minimum to avoid deploy block”, and optionally adding a separate “Warning threshold: 93%” line for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fact that this is a "warning" makes it clear. But I'm open to other wording.