-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: make newfiles part of files behaviour #573
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #573 +/- ##
=======================================
Coverage 97.49% 97.50%
=======================================
Files 414 414
Lines 34986 35002 +16
=======================================
+ Hits 34109 34127 +18
+ Misses 877 875 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #573 +/- ##
=======================================
Coverage 97.49% 97.50%
=======================================
Files 414 414
Lines 34986 35002 +16
=======================================
+ Hits 34109 34127 +18
+ Misses 877 875 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #573 +/- ##
=======================================
Coverage 97.49% 97.50%
=======================================
Files 414 414
Lines 34986 35002 +16
=======================================
+ Hits 34109 34127 +18
+ Misses 877 875 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Changes have been made to critical files, which contain lines commonly executed in production. Learn more ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #573 +/- ##
=======================================
Coverage 97.54% 97.55%
=======================================
Files 449 449
Lines 36192 36208 +16
=======================================
+ Hits 35303 35321 +18
+ Misses 889 887 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes This change has been scanned for critical changes. Learn more |
@@ -275,6 +275,14 @@ def get_upper_section_names(self, settings): | |||
if all(x not in sections for x in headers): | |||
sections.insert(0, "condensed_header") | |||
|
|||
print(sections) |
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.
and "newfiles" not in sections | ||
and "condensed_files" not in sections | ||
): | ||
sections.insert(0, "newfiles") |
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.
so we're gonna show "files" AND "newfiles" ?...
The question is: do we pop the "files" entry anywhere?
Like instead of this if
can we do sections.replace("files", "condensed_files")
?
Also let's prefer condensed_files
instead of new_files
cause that's not new anymore
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.
We indeed want to show both if files
has been selected.
If a user specifies the files formatting in their comment layout config, we want it to show the newfiles formatting as well.
this commit fixes a previously erroneous commit that attempted to make it so if "files" or "tree" are mentioned in the comment layout config then the condensed_files section is also included. We want the behaviour if "files" is selected in the config to be: 1. always show the files section 2 if there are missing lines in the path then also show the condensed_files section
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.
Left a suggestion, otherwise lgtm
af497fb
to
69639d9
Compare
If a user specifies the files formatting in their comment layout config, we want it to show the newfiles formatting as well.
Closes: codecov/engineering-team#1129