Pull Request Comment Report
Thanks for dropping by! π
We'd love your feedback about the pull request comment, focused on the comment report details, layout, and information presented.
- What do you like about the PR comment report?
- What would you'd like to see improved?
- What are challenges or frustrations you've experienced?
- 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.
Our reports changed randomly and we are unsure how to get it back to how they were before.
New:
Old:
Any advice on how to get back the more detailed report?
@bdaubry we're experimenting with a new PR comment format that we're rolling out incrementally across all users, collecting feedback as we go.
If you have any feedback on why you prefer the old PR comment to the new one, we would be glad to hear it. If you would like to revert to the old style of PR comment, you can place the following in your codecov.yml file at the root of the repository:
comment:
layout: "newheader, diff, files, newfooter"
Exactly what I needed, thank you!
If you have any feedback on why you prefer the old PR comment to the new one, we would be glad to hear it.
It's beneficial for us to see how much coverage has changed within specific files, as opposed to their total coverage. It allows us to see what might still need new test cases before a PR is merged, etc. Without the context of the previous coverage, we have to go into the builds to see the end result of the test suite, or run the tests locally with codecov.
@bdaubry thanks for that feedback. I'm a PM on the Codecov team
Out of curiosity, is the total coverage (and file specific coverage trends) on the Codecov Web UI something you've considered looking at? If you feel it's not best suited to help you with planning tests to add etc, we'd love to learn why
@bdaubry thanks for that feedback. I'm a PM on the Codecov team
Out of curiosity, is the total coverage (and file specific coverage trends) on the Codecov Web UI something you've considered looking at? If you feel it's not best suited to help you with planning tests to add etc, we'd love to learn why
It's nice that it is available there. However, we'll have PRs that are open for days to weeks, and having to go to Codecov every time would add an unnecessary step to our workflow. It was super handy having that information available in a comment on the PR that updated automatically, so anyone on our team could know exactly what files to target.
Unfortunately, the file specific trends appear to be gone from the PR comment, even following the earlier advice.
@bdaubry Can you post a screenshot of your current PR comment? I think we can get you what you need
The codecov PR comment is misleading when the changes are not covered by any tests at all. It says "All modified lines are covered by tests β " for documentation changes for example.
@bdaubry Can you post a screenshot of your current PR comment? I think we can get you what you need
Hello,
It appears between my last comment and now, our coverage deltas have returned. Current view:
I think I'm good now, I appreciate you all looking into it!
@ludofischer That's great feedback, thank you, we'll update that to be a little more relevant when the PR doesn't actually impact coverage at all - thoughts on verbiage like βThis change had no impact on code coverage.β?
@bdaubry Glad to here all is back to what you'd expect! Please let us know if you have any further issues or suggestions!
@ludofischer That's great feedback, thank you, we'll update that to be a little more relevant when the PR doesn't actually impact coverage at all - thoughts on verbiage like βThis change had no impact on code coverage.β?
I think it is definitely better to say that code coverage did not change rather than saying that the lines are covered by tests. It's a bit tricky because the situations because completely uncovered code and impossible to cover changes like docs are fairly different cases.
I really miss the diff view where I can see the converge change by lines, which gives a nice idea that is not conveyed by the percentage. I also like that it has colors (red/green) which makes it easy to scan. With PRs that touch many files a summary view is also very helpful.
Also when we update/improve only tests, coverage will increase but that will not be reflected on the changed files, because it's the source that is tested that will see the change. This increment can only be seen from the repo level coverage data.
Hi @justinchuby - we really appreciate your feedback, if you'd like to flip the comment back, you can use the below configuration in your codecov.yml:
comment:
layout: "newheader, diff, files, newfooter"
I am getting a warning that a newly added function in a julia code base is not covered, although there is a docstring attached to it. This may be because there is a comment right before the docstring?
Is this PR format simpler than it was before? Or does it just seem that way because this PR is at 100%? π§
If it's simpler I like it! :-)
P.S. There is a typo though ... double punctuation on "Let us know!."
Also this ticket I'm on has been in "Waiting for: Product Owner" for two weeks. π
Right you are @chadwhitacre -- it is simpler, and there is double punctuation.
Fixing: https://github.com/codecov/worker/pull/164
Thanks @jerrodcodecov! New simpler PR comment looks great! :-)
P.S. lolsob silly bot ... @hubertdeng123 can we convince the bot that @jerrodcodecov is not not a product owner here, and that I am not?
The comments have changed recently, and I'm struggling to get back to a similar view as before.
It seems that the diff now appears in a collapsed section, which makes it too easy to ignore and not notice any issues in coverage.
Ultimately, the one thing I need to know is whether any previously covered lines are no longer covered, or newly added lines are missing coverage (including partials). If so, then I'll just click the link to the full report to see the exact lines in the diff view (and decide if they need to be covered or not).
The diff part shows this pretty clearly, with red/green highlights at a glance. But, now it is hidden under a collapsible section it is too easy to forget about (I thought that maybe the condensed_ versions were collapsed or something, but seems they all are).
@Dreamsorcerer, thanks for the feedback! Could you share a screenshot of the report you're seeing? We want to make sure this isn't a format issue or previous version.
I need to know is whether any previously covered lines are no longer covered, or newly added lines are missing coverage (including partials). If so, then I'll just click the link to the full report
Our latest report format (below) displays any missed lines + related file (or confirms when there are none missing β ) right at the top for immediate visibility. Is this the format your seeing and referring to? If not, is this a bit closer to your preference? Would love your thoughts! π
I'm afraid that only covers direct changes in the patch:
I've updated the section in our docs with an example from a recent PR, which shows exactly how we use codecov and why this is so important (the tool itself, and being able to see the diff at a glance): https://aiohttp--7916.org.readthedocs.build/en/7916/contributing.html#tests-coverage
If indirect changes are included in that summary, then it's probably fine. But, an "all green" status or something to indicate that no coverage issues have occurred would make me feel more comfortable when there are no files to display.
Thank you @Dreamsorcerer! This is really helpful π
only covers direct changes in the patch:
That is true for now. I created an issue to capture the problem and review further with the team: https://github.com/codecov/engineering-team/issues/865
an "all green" status or something to indicate that no coverage issues
When all lines are covered, the comment summary should read "All modified and coverable lines are covered by tests β " (example). Is this similar to how you were describing?
When all lines are covered, the comment summary should read "All modified and coverable lines are covered by tests β " (example). Is this similar to how you were describing?
It's possible that I just filter it out mentally at the moment, as the information is incorrect due to not including indirect changes. I guess I'll review again when all changes are included. But, certainly the strong red/green highlighting in the diff component is very clear at a glance, so that's my baseline.
I'm not sure if this is a bug on your side or a config issue on my side, but I can't see the missing lines when I click the missing lines on a PR: https://github.com/maplibre/maplibre-gl-js/pull/3431#issuecomment-1835473074
Generally speaking, the PR comment is a lot cleaner and focused, and the missing lines is a good summary.
I'm not sure the report is up to date though as there was a passing build today, but the coverage comment is from yesterday.
If there was an option to open details in the PR comment to see some red and green lines on the relevant files (if the number of missing lines is small) it might improve productivity, i.e. similar to opening the details below the table - open details for every file to show which lines are missing.
I'm not sure if this is a bug on your side or a config issue on my side, but I can't see the missing lines when I click the missing lines on a PR:
If you look at the URLs, they all just open the page for the PR. Seems that currently you'll still need to scan through the diff to find the missed lines.
I'm not sure the report is up to date though as there was a passing build today, but the coverage comment is from yesterday.
By default the comment is updated in-place. I don't think it's always obvious enough to tell if it's up-to-date or not, but you can compare the commit mentioned in the comment to see if it's referring to the latest commit pushed or not.
If you look at the URLs, they all just open the page for the PR. Seems that currently you'll still need to scan through the diff to find the missed lines.
I'm sorry to be blunt here, but that doesn't make sense from a user experience point of view...
By default the comment is updated in-place.
I looked at the commit update time vs the last successful run time and they are not close, so I believe it wasn't updated correctly with the latest results, but I might be wrong...
I'm sorry to be blunt here, but that doesn't make sense from a user experience point of view...
I'm just another user. :P Just pointing out the current behaviour. Maybe there's improvements on the roadmap.
I looked at the commit update time
I meant the actual commit ID. From one of my PRs:
The comment mentions c473d40, and the last commit on the Github timeline shown after that comment is the same commit. So, I can tell it has been updated. Occassionally it fails to update, in which case looking at the CI run I can usually find an error message in the codecov upload step (but, it doesn't fail the CI when the upload errors).
I don't know if the codecov comments get notified by git pushes, or only get notified when an upload happens. But, if it's the former, then it'd definitely be useful for the comment to get updated immediately with ** OUTDATED ** or similar at the top of the comment.
π Also, the Edited word is a dropdown history of edits
I'll leave all the design feedback for @codecovdesign to handle :D
Although ...
I don't know if the codecov comments get notified by git pushes, or only get notified when an upload happens. But, if it's the former, then it'd definitely be useful for the comment to get updated immediately with ** OUTDATED ** or similar at the top of the comment.
Git Pushs/PR webhook events. It might not be as obvious as it cound me, but the wording in the time between the push and the upload says something to the effect of "We do not have coverage for the latest commit <SHA> on head. Please consider uploading"
Git Pushs/PR webhook events. It might not be as obvious as it cound me, but the wording in the time between the push and the upload says something to the effect of "We do not have coverage for the latest commit
<SHA>on head. Please consider uploading"
I don't see that at all (though it'd probably still be better to have something more noticeable either way):
The comment appears to be edited after every upload, but everything it displays is still referring to the previous commit (as there aren't enough uploads for the current commit yet). I didn't see an edit happen before the first upload.
Hi @Dreamsorcerer ,
Do you have the Codecov GitHub app installed? https://github.com/apps/codecov
It's possible we arn't getting the web hooks and are only updating on coverage uploads.