-
Notifications
You must be signed in to change notification settings - Fork 26
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
fix: Improve report structure, fix template issue #633
Conversation
Signed-off-by: Oleg Kopysov <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #633 +/- ##
============================================
- Coverage 93.88% 93.81% -0.08%
- Complexity 623 628 +5
============================================
Files 52 52
Lines 2127 2150 +23
Branches 247 251 +4
============================================
+ Hits 1997 2017 +20
- Misses 56 58 +2
- Partials 74 75 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd recommend to split changes to '/test' and '/src'. For one commit - it is huge change.
Tests will fail in case of changes in src/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
test will fail if different PR's used. I mean - one commit for source another commit for tests. |
What is the benefit of using such an approach? The reviewer has to review the final code, not the sequence of commits. |
it's just recommendation to simplify review. 140+ line in single commit including both source and test (I understand that it is fix) is huge. |
According to all recommendations that I found, the pull request size is recommended to be less than 200 lines. |
ok. Let's move this conversation out of this scope. |
Pull Request
Description
Current PR contains improvement of the overall report layout and some template fixes.
Type of change
Please delete options that are not relevant.
Testing
Local tests passed.
Checklist: