[
https://issues.apache.org/jira/browse/HDFS-9157?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14949932#comment-14949932
]
Mingliang Liu commented on HDFS-9157:
-------------------------------------
Thanks for the patch.
The patch overall looks good. Two comments.
# To construct a new String, there is no need to call new
String(object.toString)
{code}
+ String errorValue = new String(bytes.toString());
{code}
# It's good if you can add test case for newly changed behavior as well, if
"-h" shows up with other options together
{code}
+ if (cmd.hasOption("h")) {
+ // print help and exit with non zero exit code since
+ // it is not expected to give help and other options together.
printUsage();
- return 0;
+ return -1;
{code}
> [OEV and OIV] : Unnecessary parsing for mandatory arguements if "-h" option
> is specified as the only option
> -----------------------------------------------------------------------------------------------------------
>
> Key: HDFS-9157
> URL: https://issues.apache.org/jira/browse/HDFS-9157
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: nijel
> Assignee: nijel
> Attachments: HDFS-9157_1.patch
>
>
> In both tools, if "-h" is specified as the only option, it throws error as
> input and output not specified.
> {noformat}
> master:/home/nijel/hadoop-3.0.0-SNAPSHOT/bin # ./hdfs oev -h
> Error parsing command-line options: Missing required options: o, i
> Usage: bin/hdfs oev [OPTIONS] -i INPUT_FILE -o OUTPUT_FILE
> {noformat}
> In code the parsing is happening before the "-h" option is verified
> Can add code to return after initial check.
> {code}
> if (argv.length == 1 && argv[1] == "-h") {
> printHelp();
> return 0;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
