Skip to content
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

Configure codecov to simplify PR comments #3428

Merged
merged 1 commit into from
May 29, 2020

Conversation

travisbrown
Copy link
Contributor

@travisbrown travisbrown commented May 24, 2020

We currently use Codecov for test coverage reporting, which works nicely in general, but its default template for PR comments is much too verbose, especially since our use of property-based testing means there are often many files with small differences in coverage that show up in the report.

The result is that at the top of every PR thread there's an enormous comment that fills a screen or more and must be scrolled past to get to actual discussion.

I'm tempted to disable the PR comments entirely, and instead just have large changes in coverage fail the build. The reports would still be available on Codecov for anyone who needs them. Before we consider doing that, though, I thought we could try the smaller change here, which just removes the (often very long) reach, flags, and file sections from the PR comment. It also disables commenting when there are no changes to coverage, but I think that's unlikely to apply often because of property-based testing coverage noise.

@codecov-commenter
Copy link

Codecov Report

Merging #3428 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3428   +/-   ##
=======================================
  Coverage   91.64%   91.64%           
=======================================
  Files         381      381           
  Lines        8268     8268           
  Branches      225      225           
=======================================
  Hits         7577     7577           
  Misses        691      691           

@travisbrown
Copy link
Contributor Author

^ It's still kind of bulky but much better than e.g. the one in #3429.

@travisbrown
Copy link
Contributor Author

Also I have no idea why we got that comment given the require_changes: true.

@djspiewak djspiewak merged commit caa76fa into typelevel:master May 29, 2020
@travisbrown travisbrown added this to the 2.2.0-M3 milestone May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants