Re: Hello

[Mark Wielaard]

Hi Ingo,

On Mon, 2003-08-11 at 16:36, Ingo Prötel wrote:
> I just want to introduce myself. I have started to work for aicas GmbH .
> Currently I work on bugfixes in our VM and Classpath.
> 
> So in future you will see some bugfixes from me. I test my fixes against
> kissme, so if you have trouble with any other VM please let me know.
> 
> I'm looking forward to work with you.

You are very welcome! And thanks for testing out you patches against
another VM. That is very appreciated! Good to see that your patches are
all over the place.

There are a couple of nitpicks though. This seems to be getting a
tradition with new people joining the project... Please don't take them
personally. Nobody can come into a new project and know everything there
is to know. And even experienced hackers make mistakes from time to
time. Next time I will pick on Brian or Tom just to show that I also
dare to correct senior hackers :)

+2003-08-11 Ingo Proetel <address@hidden>
+
+       * java/security/Security.java: moved initialization code of providers
+       from static initializer into a method to allow lazy evaluation of
+       this code. This permits faster startup and even automatic removal of
+       this code if it is not needed.

ChangeLog entries should just describe the facts and be pretty dry and
factual. Just mention the method/variable that got changed, created or
deleted. The why should be in the actual code. Look at the other
ChangeLog entries for examples. A good description is at:
http://www.gnu.org/software/guile/changelogs/guile-changelogs_toc.html

So this ChangeLog entry could be written as:

        * java/security/Security.java (providers): Removed field,
        replace by method.
        (providers_lazy): New static field.
        (static): Removed and replaced by new providers() method.
        (providers): New static method, replaces static block and use of
        static field of the same name.

The ChangeLog entry should be used as the commit message and the
ChangeLog file should be committed together with the files that changed.
(As you did in your last couple of commits. Thanks)

+               /* Removed this (more comprehensive) error string to avoid the 
need for a 
+                * static variable or allocation of a buffer for this message 
in this (unlikely) 
+                * error case. --Fridi. 
+                *
+                * sprintf(errstr,"JCL: Failed to throw exception %s with 
message %s: could not find exception class.", className, errMsg); 
+                */

Just remove old, unused, commented out code. It is available in the CVS
history. No need to clutter up the source with dead code.

There is also no need to add initials or other author markers to
comments. Comments should be valid whoever wrote them. And if you want
to find out who was responsible for a certain line of code or comment
there is alway cvs annotate.

+       // ???

That is not really a useful comment.

+// NYI: ignore check until Method.getExceptionTypes is implemented             
+//             Class[] exceptions = meths[i].getExceptionTypes();
+//             int index = 0;
+//             for(;index < exceptions.length; index++){
+//                     if(exceptions[index].equals(RemoteException.class)){
+//                             break;
+//                     }
+//             }
+//             if (index < exceptions.length) {
+                       rmeths.add(meths[i]);
+//             } else {
+//                     logError("Method "+meths[i]+" does not throw a 
java.rmi.RemoteException");
+//             }
[...]
-               out.print("throws ");
+// NYI hard-code java.rmi.RemoteException until Method.getExceptionTypes is 
implemented
+//             out.print("throws ");
+               out.print("throws java.rmi.RemoteException");

This seems to be specific to VMs that don't implement
Method.getExceptionTypes(). Are you sure it is applicable in general?

Please revert this part.

Cheers,

Mark