-
Notifications
You must be signed in to change notification settings - Fork 57
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
Remove path prefix in RNP_LOG #2297
Conversation
8e1ea80
to
eef0043
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2297 +/- ##
=======================================
Coverage 84.88% 84.88%
=======================================
Files 114 114
Lines 22783 22783
=======================================
Hits 19339 19339
Misses 3444 3444 ☔ 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.
This is based on the assumption that compiler sets FILE to the file name from the command line.
I am not sure if it works for MSVC
https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170 Indeed, looks like we need /FC option for msvc. |
I think it is safe to add /FC option |
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.
Could you please add some line to cli_tests.py, which makes sure that prefix is not included. This would work for any output made with RNP_LOG, like self.assertRegex(err, r'(?s)^.*Warning: missing or malformed CRC line.*')
.
Also please rebase so all tests are run (macOS runner was renewed).
eef0043
to
d63a0e7
Compare
176dace
to
aa23507
Compare
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, thanks!
fixes #2146