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

Feedback about the new pull request comment #8

Open
kylemann opened this issue Dec 17, 2021 · 68 comments
Open

Feedback about the new pull request comment #8

kylemann opened this issue Dec 17, 2021 · 68 comments
Labels
Feedback For gathering customer feedback Waiting for: Product Owner

Comments

@kylemann
Copy link

Thanks for dropping by! 👋

We've been iterating and updating the layout, summary, and copy of the pull request comment.

  • What do you think about the changes?
  • What would you'd like to see improved?
  • What are challenges you've experienced with the report?
  • How do you use the comment today?
  • How could the report better help your workflow?
  • Any general thoughts you'd like to share!

We greatly appreciate your time and thoughts - looking forward to hearing from you ❤

Codecov team

This issue is intended to share and collect feedback about the tool. If you have support needs or questions, please see our support page.

@TechMaz
Copy link

TechMaz commented Feb 15, 2022

I think it's a great feature, but it can sometimes clog up a PR review screen, so it would be great if they were collapsible so that we can read the code around them more easily when desired.

@codecovdesign
Copy link

@TechMaz thank you for your feedback.
We are actually experimenting with a new layout, see below. It prioritizes the high level summary and then collapses the other reporting data (impacted files, table, etc).

Would love to hear your feedback 🙏

new_comment

@openrefactory
Copy link

@eliatcodecov
Please watch this issue we have open this issue one week ago still don't get any respond yet.

httpie/cli#1434

@daniellockyer
Copy link

I love it! It's so much cleaner and less obtuse <3

@unmade
Copy link

unmade commented Oct 5, 2022

I'm not sure how that's possible but pull request form shows me wrong coverage result - it says my pull request will decrease project coverage, although I'm confident my changes won't not affect coverage at all. I don't see that problem in the codecov web application.

I also compared coverage.xml files uploaded to codecov for corresponding commits and they're identical.

The pull request with wrong form:
unmade/shelf-back#97

Full report at codecov:
https://app.codecov.io/gh/unmade/shelf-back/pull/97

Screenshot 2022-10-05 at 22 22 19

And here's the coverage files for corresponding commits:

@nitram509
Copy link

Hi,

thanks for providing codecov tests to the Open Source community for free.
After adding some tests, I found your report reported lower coverage than before, when no test was there at all.
See argoproj/argo-rollouts#2303 (comment)

I do not trust the codecov report, because why it's going down when I literally create the first test file in this package?

PS: I just measured the test coverage of the new file created, this is 87% (according to IntelliJ).

@KnorpelSenf
Copy link

This may sound like a nitpick but the coverage diff has a sign error in the GitHub comment that the bot is posting. It says "coverage decreases by -3%" but decreasing by a negative amount would mean increasing. Perhaps it should say "coverage decreases: -3%" to make it more accurate?

@ssb22
Copy link

ssb22 commented Oct 29, 2022

On this pull request I simply added some lines to the documentation (README.md and README.rst), but CodeCov warned that code coverage had been reduced by my change. It would be nice if obvious documentation files didn't count. Thanks.

@mamil
Copy link

mamil commented Nov 21, 2022

Hi,

On this PR, I just copy an old case and then mod one function call, no tests were removed, and the test coverage decreased.

@sayo96
Copy link

sayo96 commented Dec 14, 2022

I'm unable to view the coverage report like before on the PR's. Codecov is such a big mess rn.

@codecovdesign
Copy link

"coverage decreases by -3%"...Perhaps it should say "coverage decreases: -3%" to make it more accurate?

Thank you, @KnorpelSenf that makes sense - will update text clarification in next iteration.

@codecovdesign
Copy link

unable to view the coverage report like before on the PR
@sayo96 it's a new PR layout we are testing. is there something specific that you noticed and/or preferred to see that was in the the previous comment?

@PaulRBerg
Copy link

PaulRBerg commented Feb 7, 2023

Is it possible to disable the automatic comments?

We'd like to keep using the Codecov GitHub app (so that our codecov.yml file gets synced with Codecov) but not receive the PR comments.

Update: it looks like comments can be disabled by adding comment: false in the Codecov config.

@slifty
Copy link

slifty commented Feb 14, 2023

Is it possible to manually request that codecov re-run the report? I've found the most spotty aspect of codecov in our development workflow to be when we force push to a branch with new test coverage but codecov doesn't update the statistics.

It would be great to be able to basically say "@codecov rerun" or something along those lines in the PR.

@ntindle
Copy link

ntindle commented Apr 25, 2023

Hey! Over at https://github.com/Significant-Gravitas/Auto-GPT/, we are running into some severe problems with CodeCov. We tried reaching out to a rep and getting some of it worked out and a demo, but they were unavailable at our scheduled time. We liked CodeCov, but it currently needs fixing and denying all PRs. I don't particularly want to swap providers, but hundreds of people are asking us why CodeCov is broken, and we need help finding answers. I would love a reply as soon as you can

@borislavnnikolov
Copy link

What about the StateHasChanged implementation for the other components, like MudTabs and etc. ?

@webbnh
Copy link

webbnh commented May 5, 2023

The pull request comment seems good to me (I'm just starting with Codecov), but there seems to be a bootstrap problem which I describe here: when there is nothing to compare to (because the base commit doesn't have any code in it yet), it's treated as a error situation instead of as though all the changes are new code (which...they are 😁).

Once my PR is merged, we'll presumably never encounter the situation again, so perhaps it's not real important, but fixing it would improve the user's on-boarding experience, and it doesn't seem like it would be that hard to address.

@codecovdesign
Copy link

@webbnh thank you for your feedback!

Once my PR is merged, we'll presumably never encounter the situation again, so perhaps it's not real important, but fixing it would improve the user's on-boarding experience

💯 that's right, the comment has a missing base report and looks like an error 🙉 . We are actively looking to have the 1st comment more welcoming and clear as seen below. If you have any thoughts or revisions you'd make let me know!

1st_time_ux

@iselo
Copy link

iselo commented Jul 15, 2023

Let's do the math!)
Screen Shot 2023-07-15 at 3 59 46 PM

@KnorpelSenf
Copy link

KnorpelSenf commented Jul 16, 2023

Those are rounded numbers, so it could have gone down by 0.008 from 99.614 to 99.606 which would be rounded to 0.01 from 99.61 to 99.61. Perhaps we would want the diff to operate based on the rounded absolute values, but that could effectively hide a decrease in test coverage.

@webbnh
Copy link

webbnh commented Jul 18, 2023

Explicit rounding aside, floating point math is much harder than it seems. (Any time you see a decimal point in the middle of the number, assume that anything to the right of it is "an estimate"! 😁)

@HarelM
Copy link

HarelM commented Mar 25, 2024

@codecovdesign is there a way to do it without introducing a whole new file to my repo in order to send a single flag to codecov? Is it possible to support the most common flag (or better yet all of them) in the github action configuration? I'm managing most of my CI configuration in this file anyway, it would be great to be able to simply add with: something or add some environment variables that the action will take into consideration.

@codecov codecov deleted a comment Mar 26, 2024
@codecov codecov deleted a comment Mar 26, 2024
@HarelM
Copy link

HarelM commented Apr 10, 2024

There's a bug that's preventing from getting accurate report due to the fact that it takes too many files (not the files I specifically requested):
The following is the relevant bug:
codecov/codecov-action#1354
I think it can be considered a critical bug...
Also I think that draft PR do not get coverage report due to some github API usage in codecov as far as I saw in the action run logs...

@codecovdesign
Copy link

@HarelM sorry missed this one; @drazisil-codecov and I were just looking at it and were unsure exactly. would you be up for a call? if so, shoot me an email at [email protected] and will find a time. curious to learn more about your use case too.

@HarelM
Copy link

HarelM commented Apr 11, 2024

I've sent a mail to see when we can meet.

@gmlewis
Copy link

gmlewis commented Apr 29, 2024

In the go-github repo, we consistently see messages like:

Report is 46 commits behind head on master.

without any explanation as to why this is (or how to fix it). As a result, the codecov report is not useful.

@thomasrockhu-codecov thomasrockhu-codecov added the Feedback For gathering customer feedback label May 1, 2024
@superlopuh
Copy link

Some time in the last few weeks, the reporting of coverage by lines with diffs stopped working, as in this PR: xdslproject/xdsl#2540
Screenshot 2024-05-05 at 17 04 44
Clearly not all lines are covered by tests, and the additional info shows this, but the message disagrees. I am also not able to see the diff when clicking through to codecov.

@mortonjt
Copy link

mortonjt commented May 13, 2024

Hi, thank you for providing this tool. We have been using this for scikit-bio

However, we have been recently having issue with codecode. Some files are mysteriously reporting little coverage, and it is not clear to us, since we have tried to create unittests that cover all of the tests (see screenshot and pull request in question here)

Screenshot from 2024-05-13 11-42-39

The only culprit that I can think of is a misconfiguration in the .codecov.yml or .coveragerc files.

Feedback would be greatly appreciated. If there is a more appropriate forum to follow up, we'll be happy to move this discussion there

@gward35
Copy link

gward35 commented May 15, 2024

Is there a way to customize the comment output so that we see top level coverage file coverage for branches/functions etc and not just total coverage? e.g.

Codecov Report

// all the typical stuff
...
Statements: 71.86%
Branch: 65.31%
Functions: 71.19%
Lines: 71.86%

I dont mind the patch showing the diff but at the end I also just want a plain output of the coverage in these respective buckets

@caendesilva
Copy link

The only reason I have Codecov is to see if a PR will add or remove coverage, as such I only care about that metric. Please default to have this open window open, or at least display the red/green diff in the main body! It used to be a lot better.

image

@codecovdesign
Copy link

thank you @caendesilva! i opened this issue to capture the feedback: codecov/engineering-team#2022. any thoughts on the update? it adds the project change to the top level summary for quick at-a-glance review.

@caendesilva
Copy link

thank you @caendesilva! i opened this issue to capture the feedback: codecov/engineering-team#2022. any thoughts on the update? it adds the project change to the top level summary for quick at-a-glance review.

Thanks! Left a comment there.

@drazisil
Copy link

@codecovdesign I feel like zero uploaded HEAD reports should be showing as a missing head report, wdyt? :D

rustymotors/rusty#32 (comment)

Image

@karfau
Copy link

karfau commented Jul 28, 2024

Here is another case where the report seems to calculate something strange: xmldom/xmldom#698 (comment)

Image

Not sure how it is calculated, but when 6 lines and one branch are being removed, they can of course also no longer be hit?

Or am I misunderstanding something?

I even added/extended tests in that commit.

@drazisil-codecov
Copy link

@karfau Not trying to discount your feedback. Is https://docs.codecov.com/docs/removed-code-behavior of help in your case?

@karfau
Copy link

karfau commented Jul 29, 2024

@drazisil-codecov if I understand this documentation correctly, the default is adjust_base which shouldn't fail the check, right?
And it looks as if I have not configured it: https://github.com/xmldom/xmldom/blob/master/codecov.yml

@drazisil-codecov
Copy link

drazisil-codecov commented Jul 29, 2024

@karfau I agree with you, after taking a look. @rohan-at-sentry is this a bug, or some part of the system that needs better docs?

https://app.codecov.io/gh/xmldom/xmldom/pull/698?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=xmldom is the only changed file, and it shouldn't fail project, given it's 100% covered.

@rohan-at-sentry
Copy link

rohan-at-sentry commented Jul 29, 2024

@karfau - we'll look into why adjust_base isn't quite working well in your scenario. In the meantime, you can set the removed code behavior to allow a fully covered patch that removes lines to be accepted. You can edit your codecov.yml with the following setting

coverage:
  status:
    project:
      default:
        target: auto
        removed_code_behavior: fully_covered_patch 

@drazisil-codecov
Copy link

Created #463 to investigate

gmlewis added a commit to gmlewis/go-github that referenced this issue Jul 29, 2024
@moi90
Copy link

moi90 commented Sep 7, 2024

I get comments by codecov bot on PRs that were already merged a long time ago, like this one: morphocut/morphocut#105 (comment)

@rohan-at-sentry
Copy link

@moi90 thanks for raising. It seems like it's related to some recent improvements we made to the notification service. We're tracking this here (#497) if you want to follow along.

@tiltowait
Copy link

codecov is great, but the report is always telling me to install the app, even though I'm pretty sure it is? Do I need to install it on all repos to suppress this message? I only need it on the one for now.

Report screenshot:
Image

Github settings screenshot:
Image

@Hugh-888
Copy link

Hugh-888 commented Nov 5, 2024

Bug report on PR #388

details: unable to import piquasso on Windows OS, since missing piquasso._math.torontonian.py file
Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feedback For gathering customer feedback Waiting for: Product Owner
Projects
Status: Waiting for: Product Owner
Development

No branches or pull requests