hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kengo Seki (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11903) test-patch should fail any new classes called Default-foo
Date Sat, 04 Jul 2015 17:25:05 GMT

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

Kengo Seki commented on HADOOP-11903:
-------------------------------------

Attached a patch.

Adding Default-foo is detected as follows (showing only related parts):

{code}
[sekikn@mobile hadoop]$ cat ~/add_default.patch 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DefaultUtil.java
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DefaultUtil.java
new file mode 100644
index 0000000..ca3be96
--- /dev/null
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/DefaultUtil.java
@@ -0,0 +1,20 @@

(snip)

+package org.apache.hadoop.util;
+
+public interface DefaultUtil { }
[sekikn@mobile hadoop]$ dev-support/test-patch.sh --basedir=/Users/sekikn/hadoop --project=hadoop
--resetrepo ~/add_default.patch 

| Vote |      Subsystem |  Runtime   | Comment
============================================================================
|  -1  |     classname  |  0m 00s    | The patch has 1 new problematic 
|      |                |            | classnames.

|| Subsystem || Report/Notes ||
============================================================================
| classname | /private/tmp/test-patch-hadoop/70393/bad-classname.txt |
{code}

But modifying existing Default-foo is not considered to be a problem:

{code}
[sekikn@mobile hadoop]$ cat ~/modify_default.patch 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/DefaultStringifier.java
b/hadoop-common-project/hadoopcommon/src/main/java/org/apache/hadoop/io/DefaultStringifier.java
index 3ba577f..8b0bfd5 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/DefaultStringifier.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/DefaultStringifier.java
@@ -203,5 +203,4 @@ public void close() throws IOException {
       stringifier.close();
     }
   }
-
 }
[sekikn@mobile hadoop]$ dev-support/test-patch.sh --basedir=/Users/sekikn/hadoop --project=hadoop
--resetrepo ~/modify_default.patch 

| Vote |      Subsystem |  Runtime   | Comment
============================================================================
|  +1  |     classname  |  0m 00s    | Patch has no classname issues. 
{code}

Of course, unrelated changes are not regarded as problems:

{code}
[sekikn@mobile hadoop]$ cat ~/add_new.patch 
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/NewClass.java
b/hadoop-common-project/hadoopcommon/src/main/java/org/apache/hadoop/NewClass.java
new file mode 100644
index 0000000..60c2057
--- /dev/null
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/NewClass.java
@@ -0,0 +1,20 @@

(snip)

+package org.apache.hadoop;
+
+public class NewClass { }
[sekikn@mobile hadoop]$ dev-support/test-patch.sh --basedir=/Users/sekikn/hadoop --project=hadoop
--resetrepo ~/add_new.patch 

| Vote |      Subsystem |  Runtime   | Comment
============================================================================
|  +1  |     classname  |  0m 00s    | Patch has no classname issues. 
{code}

Users can specify the patterns which are regarded as invalid classnames:

{code}
[sekikn@mobile hadoop]$ dev-support/test-patch.sh --basedir=/Users/sekikn/hadoop --project=hadoop
--resetrepo --classname-re='^Default|^New' ~/add_new.patch 

| Vote |      Subsystem |  Runtime   | Comment
============================================================================
|  -1  |     classname  |  0m 00s    | The patch has 1 new problematic 
|      |                |            | classnames.

|| Subsystem || Report/Notes ||
============================================================================
| classname | /private/tmp/test-patch-hadoop/25538/bad-classname.txt |

[sekikn@mobile hadoop]$ cat /private/tmp/test-patch-hadoop/25538/bad-classname.txt
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/NewClass.java
{code}

> test-patch should fail any new classes called Default-foo
> ---------------------------------------------------------
>
>                 Key: HADOOP-11903
>                 URL: https://issues.apache.org/jira/browse/HADOOP-11903
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: yetus
>    Affects Versions: HADOOP-12111
>            Reporter: Allen Wittenauer
>            Assignee: Kengo Seki
>         Attachments: HADOOP-11903.HADOOP-12111.00.patch
>
>
> In the past, we've named things like DefaultResourceCalculator, DefaultContainerExecutor,
and DefaultCodec that do nothing but cause problems down the road since they are effectively
version and functionality locked forever.  If these examples had been named what they truly
were (e.g., MemoryResourceCalculator, SimpleContainerExecutor, and GZipCodec), the defaults
could then be changed in the future in a compatible way. 
> One way to enforce this is to prevent the creation of new classes called Default-anything.




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message