[
https://issues.apache.org/jira/browse/DERBY-2311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477130
]
Andrew McIntyre commented on DERBY-2311:
----------------------------------------
A few comments on these patches:
It appears the patches were generated down in the system test hierarchy. It's
better for the patches to be generated at the top of the tree, reviewers expect
them to be generated that way, plus it captures all the changes in your
checkout in one patch file. It would have been easier to review this patch if
it had been together in one file. Also, there are a lot of formatting changes
in the file. Formatting changes without any content change clutter the patch
and make it harder to review. Avoid unnecessary formatting changes if possible.
Take a look at http://wiki.apache.org/db-derby/PatchAdvice for some tips on
creating patches that are easy for reviewers to review.
DbTasks.java:
You import these three files, but then never use them:
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.jar.JarInputStream;
Several places in the file are invocations of Random.nextInt that appear as so:
Rn.nextInt( 102400 - 0 + 1 ) + 0;
While it's obvious that it getting a random size for the stream, it would be
good to add a comment as to why this size was chosen for this object. Also,
there isn't a need for the addition and subtraction here, although in some of
the other cases where this appears, a number is added after the random number
is generated. Please condense the addition and subtraction in these cases
and/or add a comment as to why they are necessary.
Datatypes.java:
Same comment regarding the use of Random as above.
There were a lot of System.out.printlns removed, was that intentional?
Minor thing, but in both cases you create a new Random object and continue to
use Math.random(). Seems like you could standardize generating your random
numbers on one of the two methods.
> Generate the txt and blobs required for the system tests dynamically
> ----------------------------------------------------------------------
>
> Key: DERBY-2311
> URL: https://issues.apache.org/jira/browse/DERBY-2311
> Project: Derby
> Issue Type: Sub-task
> Components: Test
> Affects Versions: 10.3.0.0
> Reporter: Manjula Kutty
> Assigned To: Manjula Kutty
> Priority: Minor
> Fix For: 10.3.0.0
>
> Attachments: DERBY-2311_diff.txt, DERBY-2311_diff2.txt,
> DERBY-2311_stat.txt, DERBY-2311_stat2.txt, DERBY-2313_diff.txt
>
>
> Generate the txt and blobs dynamically . So that the size of the jar file
> will get reduced. And no need of a seperate resource.jar file
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
