-
Notifications
You must be signed in to change notification settings - Fork 9
QUA-360: Add skip diff coverage method #152
Conversation
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.
Looks good! Just one small doubt
end | ||
|
||
response | ||
receive_request(%w[pending success failure skipped error], :update_status) |
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.
Is there a reason to pass the call_method
as a symbol instead of a string? I see that we convert it as a string later in the receive_request
method
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.
Honestly, there isn't a strong reason. I just felt like it was more readable, but I'm open to change it if you don't feel the same about it
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 personally like it as a symbol. I associate symbols with methods better 👍
@@ -36,6 +36,7 @@ def self.load_services | |||
issue | |||
pull_request | |||
pull_request_coverage |
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.
Just curious: is this the Actually, never mind.total_coverage
event?
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.
Were you able to find out why this is requested?? It's the available services names, so you can call it like
service = CC::Service::GitHubPullRequests.new(
{
oauth_token: ...,
},
name: "pull_request_diff_coverage",
state: "skipped",
github_slug: ...,
number: ...,
commit_sha: ...
)
Context
Now that we're implementing the ability to skip draft PRs reviews, we want to avoid the "waiting for status to be reported" situation on the diff and total coverage statuses, specifically when you configured them as a protection branch rule.
Changes
CC::PullRequests
classRelated issue
https://codeclimate.atlassian.net/browse/QUA-360