-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add checking the uss file path when upload file or stdin to uss file. #145
Conversation
Signed-off-by: Tian Na <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #145 +/- ##
==========================================
+ Coverage 72.39% 73.04% +0.64%
==========================================
Files 79 80 +1
Lines 960 983 +23
Branches 125 130 +5
==========================================
+ Hits 695 718 +23
Misses 244 244
Partials 21 21 ☔ View full report in Codecov by Sentry. |
params.response.data.setMessage(successMsg); | ||
this.log.info(successMsg); | ||
|
||
if (!ussFile.startsWith('/')) { |
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.
Good to have this check in UssUtils.checkAbsoluteFilePath(filePath)
, which returns error message when the check fails. In this way, we can avoid having the check and error message duplicate in two places.
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.
Thank you @std4lqi ! I add checkAbsoluteFilePath in UssUtils and add test in UssUtils.test.ts.
Signed-off-by: Tian Na <[email protected]>
Signed-off-by: Tian Na <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Thanks, @tiantn ! |
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! 😋
if (!filePath.startsWith('/')) { | ||
throw new ImperativeError({ msg: "Please check the uss file path. The full file path is required."}); | ||
} else { | ||
return filePath; |
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.
Based on the current usage of the function, I don't think we need to return the filePath, but it doesn't hurt (in case we want to use it in the near future)
No changes requested here 😋
Release succeeded for the The following packages have been published:
Powered by Octorelease 🚀 |
What It Does
How to Test
Review Checklist
I certify that I have:
Additional Comments
fix #144