Re: Mauve patches.

[Andrew Haley <aph@redhat.com>]

Thomas Zander writes:
 > -----BEGIN PGP SIGNED MESSAGE-----
 > Hash: SHA1
 > 
 > On Tuesday 13 April 2004 19:44, Andrew Haley wrote:
 > > I have attached a version here, FYI.
 > >
 > > I don't think I've broken anything with the checkin, but please check.
 > 
 > Hmm; I'm wondering what was wrong with the last patch I sent; the only 
 > difference I see is that I failed to mention the new method in 
 > SimpleTestHarness, and the change in statics in 
 > SimpleDateFormat/attribute.java.

This is what you sent:

diff -U3 -p -N -r mauve-orig/ChangeLog mauve/ChangeLog
--- mauve-orig/ChangeLog        2004-04-03 17:59:25.000000000 +0200
+++ mauve/ChangeLog     2004-04-03 17:54:08.000000000 +0200
@@ -1,4 +1,17 @@
+2004-04-03  Thomas Zander <zander@kde.org>
+
+       * new files
+       gnu/testlet/javax/swing/JLabel/Icon.java,
+       gnu/testlet/javax/swing/JLabel/Mnemonic.java
+
+2004-04-03  Thomas Zander <zander@kde.org>
+
+       * added an ant build option so the autotools are not needed if you use
+       a fully functional JVM (for example for writing tests).
+       build.xml:  ant build file
+       gnu/anttask/RunTests.java: ant task for calling SimpleTestHarness.java
+

This is what I did to make it right:

2004-04-13  Thomas Zander <zander@kde.org>

        * build.xml: New file.
        * gnu/anttask/RunTests.java: New file.
        * gnu/testlet/SimpleTestHarness.java (getFailures): New method.

        * gnu/testlet/javax/swing/JLabel/Icon.java: New file.
        * gnu/testlet/javax/swing/JLabel/Mnemonic.java: New file.

        * gnu/testlet/java/text/SimpleDateFormat/attribute.java 
        (test_FieldPos): Locals no longer static.

See http://www.gnu.org/prep/standards_42.html#SEC42. 

Note in particular that Change Logs document only what, not why.
Explanations should be comments in the program.

 > Was it a bad choice to sent it as a bzip2 compressed patch?

Not always, but it does mean that reviewers are less likely to look at
your patch straight away.

 > Anyway; you forgot this thingy:
 > 
 > diff -U3 -p -N -r mauve-orig/.cvsignore mauve-new/.cvsignore

Ah, yes.  Thanks.

Andrew.