hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Yongjun Zhang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11293) Factor OSType out from Shell
Date Thu, 13 Nov 2014 17:24:34 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-11293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14210045#comment-14210045

Yongjun Zhang commented on HADOOP-11293:

Hi Steve,

Many thanks for the review and comments. 

I agree with you that the preserving the original flag name can avoid some typing. However,
adding {{IS_}} to the flag seems to make a better name (I hope you agree:-)).

What I did for this change was to run a shell command
 find . -name "*.java" -exec sed -i 's/Shell\.WINDOWS/CurrentOS\.IS_WINDOWS/g' {} \;
that changed all files. So it did not cause me much trouble. If developer is editing one file,
a search/replace would do the same. Since we are introducing this new public interface, it's
an opportunity to make this change in the beginning. Once it's there, then it will be harder
to change later. I personally think it's worthwhile to have the new name. But if you think
the benefit of avoiding some typing is better, I'm flexible here. Would you please let me
know again if it's ok with you to have the {{IS_}}? If not, I can change to use the same flag
names as before. 

What I had to spend time is to open each file to fix the import. My eclipse does automatically
insert an empty line in between different import sections (which only happens when there is
any change to be done in the import area). I thought this empty-line change actually helps
readability, so I did not try to revert it earlier. If it indeed cause merge trouble, I would
agree that it's better not to have them. And I will go remove them. 

Thanks again!

> Factor OSType out from Shell
> ----------------------------
>                 Key: HADOOP-11293
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11293
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: util
>            Reporter: Yongjun Zhang
>            Assignee: Yongjun Zhang
>         Attachments: HADOOP-11293.001.patch, HADOOP-11293.002.patch
> Currently the code that detects the OS type is located in Shell.java. Code that need
to check OS type refers to Shell, even if no other stuff of Shell is needed. 
> I am proposing to refactor OSType out to  its own class, so to make the OSType easier
to access and the dependency cleaner.

This message was sent by Atlassian JIRA

View raw message