Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

QUA-360: Add skip diff coverage method #152

Merged
merged 3 commits into from
Jun 30, 2022

Conversation

camillof
Copy link
Contributor

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

  • Introduce a new method to report a skipped diff/total coverage status
  • Done some refactoring (which can be ignored if we don't like it)
  • Added tests for the CC::PullRequests class

Related issue

https://codeclimate.atlassian.net/browse/QUA-360

@camillof camillof requested review from a team, f-moya and dantevvp and removed request for a team June 29, 2022 14:08
Copy link

@dantevvp dantevvp left a 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)

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

Copy link
Contributor Author

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

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
Copy link
Contributor

@larkinscott larkinscott Jun 29, 2022

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 total_coverage event? Actually, never mind.

Copy link
Contributor Author

@camillof camillof Jun 29, 2022

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: ...
)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants