[ 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)