[ 
https://issues.apache.org/jira/browse/HDFS-6328?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13991271#comment-13991271
 ] 

Jing Zhao commented on HDFS-6328:
---------------------------------

bq. Is there a reason why this loop needed to become more complicated? At this 
point I believe it's guaranteed that the src & dest are not identical, nor is 
the src a subdir of the dest?
Good catch, [~daryn]. I should catch this in my first review.

bq. As a general statement, I'm not sure there's a lot of value add in the 
changes like altering whitespace and moving methods. 
Although some changes like changing for loop to while loop is not necessary, I 
think this is still good cleanup, especially for those changes in the two 
unprotectedRename methods. One more thing we should do is to put common code of 
the two unprotectedRename methods into separate methods. But I'm fine with 
doing this in a separate jira.

For the latest patch, the following change is unnecessary:
{code}
-        src.endsWith(HdfsConstants.SEPARATOR_DOT_SNAPSHOT_DIR), 
-        "%s does not end with %s", src, 
HdfsConstants.SEPARATOR_DOT_SNAPSHOT_DIR);
+            src.endsWith(HdfsConstants.SEPARATOR_DOT_SNAPSHOT_DIR),
+            "%s does not end with %s", src, 
HdfsConstants.SEPARATOR_DOT_SNAPSHOT_DIR);
{code}
Also, maybe we do not need to move the set of getINode methods. As [~daryn] 
pointed out, these changes can make merging harder (e.g., merging changes from 
2.x to 0.23).


> Simplify code in FSDirectory
> ----------------------------
>
>                 Key: HDFS-6328
>                 URL: https://issues.apache.org/jira/browse/HDFS-6328
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: namenode
>            Reporter: Haohui Mai
>            Assignee: Haohui Mai
>         Attachments: HDFS-6328.000.patch, HDFS-6328.001.patch, 
> HDFS-6328.002.patch
>
>
> This jira proposes:
> # Cleaning up dead code in FSDirectory.
> # Simplify the control flows that IntelliJ flags as warnings.
> # Move functions related to resolving paths into one place.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to