-
Notifications
You must be signed in to change notification settings - Fork 30
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 OutputFileHandler not ignoring a correctly named gz filename #2989
Fix OutputFileHandler not ignoring a correctly named gz filename #2989
Conversation
Refactor a method for better readability Fix missing '#' in changelog #2800
[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed! |
[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed! |
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! 🚀
@@ -19,16 +19,20 @@ object OutputFileHandler { | |||
} | |||
|
|||
fun checkAndFixFileExtension(outputName: String): String { | |||
if (outputName.endsWith("cc.json")) { | |||
if (outputName.endsWith("cc.json") || outputName.endsWith(".cc.json.gz")) { |
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 would just use a regular expression for cases like these. That way it's a single check with optional parts. I would also handle the .cc
optional (and our logic here is somewhat flawed as it would match files like abcc.json
. I would accept it as it's a json file but it's just inconsistent right now.
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 think I remember @ce-bo saying that we should only consider .cc.json as valid, but thanks a lot for the hint with the regEx!
@phanlezz also noticed that we append .gz again, even if the user already specified it (so test.cc.json.gz becomes test.cc.json.gz.gz)
Should we/I open a new ticket for this or continue this here?
Fix OutputFileHandler not ignoring a correctly named gz filename
Issue: #2800
Description
Addendum to PR #2914 to fix a bug that was discovered after the merge :(