Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: None
    • Labels:
      None

      Description

      Common portion of the MAPREDUCE-1545.

      1. hadoop-6657-trunk-v3.patch
        3 kB
        Luke Lu
      2. hadoop-6657-trunk-v2.patch
        3 kB
        Luke Lu
      3. hadoop-6657-trunk-v1.patch
        2 kB
        Luke Lu

        Issue Links

          Activity

          Hide
          Luke Lu added a comment -

          Thanks! Steve.

          Show
          Luke Lu added a comment - Thanks! Steve.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #221 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/221/)
          HADOOP-6657. Add a capitalization method to StringUtils for MAPREDUCE-1545

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #221 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/221/ ) HADOOP-6657 . Add a capitalization method to StringUtils for MAPREDUCE-1545
          Hide
          Steve Loughran added a comment -

          I've just checked in the change, but JIRA isn't giving me permission to work on the issue, to mark it as fixed.

          Show
          Steve Loughran added a comment - I've just checked in the change, but JIRA isn't giving me permission to work on the issue, to mark it as fixed.
          Hide
          Steve Loughran added a comment -

          +1

          The i18n problems I worry about seem addressed with tests and if commons-lang was in there already (probably some random transitive use) then there's less code to worry about.

          Show
          Steve Loughran added a comment - +1 The i18n problems I worry about seem addressed with tests and if commons-lang was in there already (probably some random transitive use) then there's less code to worry about.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12439743/hadoop-6657-trunk-v3.patch
          against trunk revision 927193.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12439743/hadoop-6657-trunk-v3.patch against trunk revision 927193. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/431/console This message is automatically generated.
          Hide
          Luke Lu added a comment -

          Just noticed that commons lang is already in trunk, though it doesn't seem to be used by any code, until now.

          Show
          Luke Lu added a comment - Just noticed that commons lang is already in trunk, though it doesn't seem to be used by any code, until now.
          Hide
          Luke Lu added a comment -

          Thanks for the comment, Steve and Tom.

          Hopefully v2 is a little more robust wrt to locales.

          @Tom, I was aware of common's capitalize. But introducing yet another dependency (in at least two projects, editing at least 4 xml files) for a trivial 3 line function doesn't seem worth it, especially before HADOOP-6629. And I prefer it throwing NPE instead of return null. At least it's now 100% compatible (use toTitleCase) for non-null case.

          Show
          Luke Lu added a comment - Thanks for the comment, Steve and Tom. Hopefully v2 is a little more robust wrt to locales. @Tom, I was aware of common's capitalize. But introducing yet another dependency (in at least two projects, editing at least 4 xml files) for a trivial 3 line function doesn't seem worth it, especially before HADOOP-6629 . And I prefer it throwing NPE instead of return null. At least it's now 100% compatible (use toTitleCase) for non-null case.
          Show
          Tom White added a comment - Commons Lang has a capitalize method: http://commons.apache.org/lang/api/org/apache/commons/lang/StringUtils.html#capitalize%28java.lang.String%29
          Hide
          Steve Loughran added a comment -

          If you are going to do toUpper/toLower then you need to handle odd locales where the conversion logic doesn't match EN_US. Easiest way: Specify the locale in the conversion operation. Otherwise you end up fielding bugreps from places like turkey that you can't replicate locally.

          Add a test for the entire ascii alphabet convertiing just to make sure all is well.

          Show
          Steve Loughran added a comment - If you are going to do toUpper/toLower then you need to handle odd locales where the conversion logic doesn't match EN_US. Easiest way: Specify the locale in the conversion operation. Otherwise you end up fielding bugreps from places like turkey that you can't replicate locally. Add a test for the entire ascii alphabet convertiing just to make sure all is well.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12439624/hadoop-6657-trunk-v1.patch
          against trunk revision 926421.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12439624/hadoop-6657-trunk-v1.patch against trunk revision 926421. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/37/console This message is automatically generated.

            People

            • Assignee:
              Luke Lu
              Reporter:
              Luke Lu
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development