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
Improve mc-cp error message #2208
Conversation
Sample output:
|
Codecov Report
@@ Coverage Diff @@
## master #2208 +/- ##
======================================
Coverage 9.02% 9.02%
======================================
Files 93 93
Lines 6902 6902
======================================
Hits 623 623
Misses 6152 6152
Partials 127 127
Continue to review full report at Codecov.
|
cmd/cp-url-syntax.go
Outdated
@@ -42,7 +43,7 @@ func checkCopySyntax(ctx *cli.Context) { | |||
for _, srcURL := range srcURLs { | |||
_, _, err := url2Stat(srcURL) | |||
if err != nil { | |||
fatalIf(err.Trace(srcURL), fmt.Sprintf("Unable to stat '%s'.", srcURL)) | |||
console.Fatalf("Source %s doesn't exist.\n", srcURL) |
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.
Couldn't this be sometimes a network problem ?
We probably can find a better phrase than 'Unable to stat' of this latter is considered not user friendly
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.
Suggestion: "Unable to determine validity of source %s"
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.
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.
Posted one comment, need to be discussed
cmd/cp-url-syntax.go
Outdated
@@ -42,7 +43,7 @@ func checkCopySyntax(ctx *cli.Context) { | |||
for _, srcURL := range srcURLs { | |||
_, _, err := url2Stat(srcURL) | |||
if err != nil { | |||
fatalIf(err.Trace(srcURL), fmt.Sprintf("Unable to stat '%s'.", srcURL)) | |||
console.Fatalf("Unable to determine validity of source %s\n", srcURL) |
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.
Unable to validate source %s
or
Unable to check source %s
or
Unable to locate source %s
I think these would be better. I'll approve if you vote for no.
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 like the first 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.
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
Fixes #2195