codecov bot

[est31]

Can we disable the codecov bot? I'm seeing it spamming PRs like #185 with information that is extremely useless. I don't think I've ever found any comment of that bot useful. It's fine if we have a codecov link somewhere with a graph or such, but the github integration is a nuisance.

[cpu]

I'm seeing it spamming PRs like #185

I'm not sure I see the spam. There's only one comment from the bot on that PR.

with information that is extremely useless.

Why do you think the information is useless? I find the summary of coverage changes helpful: it's possible to see all of the changes to coverage across all of the touched files without having to go to a separate service/browser tab.

[est31]

I mean the stuff in the diff tab:

Why do you think the information is useless?

I don't really think that chasing some coverage number goal is important. Tests are important, the number isn't. You can make shitty tests that have high coverage numbers, and you can make good tests that increase it by little only. If a PR is adding no tests even though it should add them then you can see that by inspecting the diff, so you see it regardless of whether many lines are added or whether it's just few lines. I always scroll over the codecov bot's comment in the thread, but if it's helpful to you folks we can keep it.

My bigger problem though is with the comments it adds to the diff of the newly added lines (see screenshot): some derive impls not being covered by tests, that's info that I don't care about. A comment on github is an extremely high level of information, and I just want to inspect the diff.

[cpu]

I mean the stuff in the diff tab

Ah ok. I see what you're talking about. Thanks, a screenshot makes this a lot clearer.

a) I don't really think that chasing some coverage number goal is important

I think you're making a big assumption that this is my motivation. It definitely isn't, which is also why there's no required coverage goal configured.

Tests are important, the number isn't.

I agree tests are important. This is a tool for seeing what your tests are and aren't testing. You can try and do that by visual inspection but this is time consuming and error prone. In my experience it's easy to think a test is covering certain branches when it isn't. The comments are meant to be making that explicit and easier to spot in the code review context.

You can make shitty tests that have high coverage numbers

Have you seen that happen with PRs being submitted to the repo? That should be review feedback. Let's not add shitty tests 😆

and you can make good tests that increase it by little only.

I think that kind of test won't be a problem because we're not requiring coverage anywhere. If new code being proposed in a PR is only covered a little bit that's something worth discussing in review but I don't think anyone is being hard-line about that.

I always scroll over the codecov bot's comment in the thread, but if it's helpful to you folks we can keep it.

I'm fine changing it, but want to do it based on real arguments for/against.

My bigger problem though is with the comments it adds to the diff of the newly added lines (see screenshot)

I agree the signal/noise on those comments isn't very good for a codebase like this one based on the changes we're making in most PRs. Let's turn em off.

[cpu]

Let's turn em off.

#187

I'm not sure if that will kill the diff comments as well, hopefully it does. If not I'll research further.

[est31]

Thanks @cpu for addressing this!

[cpu]

I'm seeing the bot leaving coverage comments again (e.g. https://github.com/rustls/rcgen/pull/223/files). Reopening so I can remember to remove the bot or fix it again.

[cpu]

I'm seeing the bot leaving coverage comments again

I think my original fix never worked for addressing the annotations from the screenshot. The setting we toggled in #187 was for the comment the bot can leave with overall coverage information (like this) and not the annotations. I think #225 will do the trick.