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
    • Hadoop Flags:
      Reviewed

      Description

      There are some normal java methods defiend in .jsp files. It would be easier if these methods are moved to normal .java files.

      1. 5857_20090518.patch
        113 kB
        Tsz Wo Nicholas Sze
      2. 5857_20090519.patch
        113 kB
        Tsz Wo Nicholas Sze

        Activity

        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have committed this.

        Show
        Tsz Wo Nicholas Sze added a comment - I have committed this.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I checked the web pages manually. Everything works fine.

        Show
        Tsz Wo Nicholas Sze added a comment - I checked the web pages manually. Everything works fine.
        Hide
        Suresh Srinivas added a comment -

        Patch looks good +1

        Show
        Suresh Srinivas added a comment - Patch looks good +1
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Note that, before applying the patch, run

        svn mv src/hdfs/org/apache/hadoop/hdfs/server/namenode/JspHelper.java src/hdfs/org/apache/hadoop/hdfs/server/common/JspHelper.java
        cp src/hdfs/org/apache/hadoop/hdfs/server/common/JspHelper.java src/hdfs/org/apache/hadoop/hdfs/server/namenode/JspHelper.java

        After applied the patch, do svn add and svn rm.

             [exec] -1 overall.  
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
             [exec]                         Please justify why no tests are needed for this patch.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        

        This is a code refactoring. No new tests added.

        Show
        Tsz Wo Nicholas Sze added a comment - Note that, before applying the patch, run svn mv src/hdfs/org/apache/hadoop/hdfs/server/namenode/JspHelper.java src/hdfs/org/apache/hadoop/hdfs/server/common/JspHelper.java cp src/hdfs/org/apache/hadoop/hdfs/server/common/JspHelper.java src/hdfs/org/apache/hadoop/hdfs/server/namenode/JspHelper.java After applied the patch, do svn add and svn rm. [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. This is a code refactoring. No new tests added.
        Hide
        Tsz Wo Nicholas Sze added a comment - - edited

        5857_20090519.patch:

        1. removed the extra +
        2. colNum was removed since it was not used
        3. reformatted the comments. Also reformatted some codes for better readability.

        Suresh, thanks for the comments.

        Show
        Tsz Wo Nicholas Sze added a comment - - edited 5857_20090519.patch: removed the extra + colNum was removed since it was not used reformatted the comments. Also reformatted some codes for better readability. Suresh, thanks for the comments.
        Hide
        Suresh Srinivas added a comment -

        Ignore my comment (4)...

        Show
        Suresh Srinivas added a comment - Ignore my comment (4)...
        Hide
        Suresh Srinivas added a comment -

        This is good refactoring. Some comments:

        1. DatanodeJspHelper.generateFileDetails() - while forming downloadURL there is an extra +
        2. NodeListJsp.counterReset() does not set colNum to zero?
        3. NodeListJsp.generateNodeData initial comments could be more readable if the bullets are in separate line
        4. Not sure where the code related some of the methods from ...namenode.JspHelper is?
        Show
        Suresh Srinivas added a comment - This is good refactoring. Some comments: DatanodeJspHelper.generateFileDetails() - while forming downloadURL there is an extra + NodeListJsp.counterReset() does not set colNum to zero? NodeListJsp.generateNodeData initial comments could be more readable if the bullets are in separate line Not sure where the code related some of the methods from ...namenode.JspHelper is?
        Hide
        Tsz Wo Nicholas Sze added a comment -

        5857_20090518.patch:

        • moved namenode.JspHelper to common.JspHelper
        • moved namenode jsp codes to a new class namenode.NamenodeJspHelper
        • moved datanode jsp codes to a new class datanode.DatanodeJspHelper
        Show
        Tsz Wo Nicholas Sze added a comment - 5857_20090518.patch: moved namenode.JspHelper to common.JspHelper moved namenode jsp codes to a new class namenode.NamenodeJspHelper moved datanode jsp codes to a new class datanode.DatanodeJspHelper

          People

          • Assignee:
            Tsz Wo Nicholas Sze
            Reporter:
            Tsz Wo Nicholas Sze
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development