Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-742

Improve the java comments for the π examples

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.21.0
    • Fix Version/s: 0.21.0
    • Component/s: documentation, examples
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      There are 3 examples, pi, bbp and distbbp for π computation. We should tell the difference between them.

      1. m742_20090710.patch
        7 kB
        Tsz Wo Nicholas Sze
      2. m742_20090709.patch
        7 kB
        Tsz Wo Nicholas Sze
      3. m742_20090708.patch
        7 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 -
             [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 new tests are needed for this patch.
             [exec]                         Also please list what manual steps were performed to verify 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 release audit.  The applied patch does not increase the total number of release audit warnings.
        

        The patch are mostly documentation changes. The code changes are all related to output messages. So I did not add new unit tests.

        Tested Util.millis2String(long) manually as shown below:

          public static void main(String[] args) {
            long n = (1L << 32) - 1;
            System.out.println(Util.millis2String(n));
          }
        

        Before the patch, the output is "49 days 17:02:47.-01" which is incorrect.
        After the patch, the correct value, "49 days 17:02:47.295", is printed.

        Show
        Tsz Wo Nicholas Sze added a comment - [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 new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify 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 release audit. The applied patch does not increase the total number of release audit warnings. The patch are mostly documentation changes. The code changes are all related to output messages. So I did not add new unit tests. Tested Util.millis2String(long) manually as shown below: public static void main( String [] args) { long n = (1L << 32) - 1; System .out.println(Util.millis2String(n)); } Before the patch, the output is "49 days 17:02:47.-01" which is incorrect. After the patch, the correct value, "49 days 17:02:47.295", is printed.
        Hide
        Jakob Homan added a comment -

        Looks great +1.

        Show
        Jakob Homan added a comment - Looks great +1.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Thanks, Jakob for the review comments.

        m742_20090710.patch: rewrote the paragraphs about the limitation.

        Show
        Tsz Wo Nicholas Sze added a comment - Thanks, Jakob for the review comments. m742_20090710.patch: rewrote the paragraphs about the limitation.
        Hide
        Jakob Homan added a comment -

        Looks good. +1 with a couple nits: "emphasize on" should be just emphasize (line 23). Also, the comment on IMPLEMENTATION_LIMIT seems a bit misleading. Would something like: "Maximum digit to compute" emphasize that the value is a goal, rather than a limitation of the actual implementation?

        Show
        Jakob Homan added a comment - Looks good. +1 with a couple nits: "emphasize on" should be just emphasize (line 23). Also, the comment on IMPLEMENTATION_LIMIT seems a bit misleading. Would something like: "Maximum digit to compute" emphasize that the value is a goal, rather than a limitation of the actual implementation?
        Hide
        Tsz Wo Nicholas Sze added a comment -

        m742_20090709.patch: slightly changed the comments.

        Show
        Tsz Wo Nicholas Sze added a comment - m742_20090709.patch: slightly changed the comments.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        m742_20090708.patch:

        • added comments to pi, bbp and distbbp.
        • fix a bug in converting milliseconds to strings.
        Show
        Tsz Wo Nicholas Sze added a comment - m742_20090708.patch: added comments to pi, bbp and distbbp. fix a bug in converting milliseconds to strings.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development