Re: [cp-patches] RFC: GtkImage patch.

[Michael Koch]

On Thu, Jun 02, 2005 at 12:23:32PM +0200, Sven de Marothy wrote:
> On Wed, 2005-06-01 at 18:30 -0400, Thomas Fitzsimmons wrote:
> > >   /**
> > >    * This is an old method, no idea if it's correct.
> > >    */
> > >   private int[] convertPixels (byte[] pixels)
> > 
> > Can this comment be removed?
> 
> Only if you can confirm the method is correct. I'm still not sure it is.
> It's just what the old code did.
> 
> > > /* functions used by other gtk peer native routines */
> > > GdkPixbuf *gnu_java_awt_peer_gtk_GtkImage_getPixbuf(JNIEnv *env, jobject 
> > > obj);
> > > GdkPixmap *gnu_java_awt_peer_gtk_GtkImage_getPixmap(JNIEnv *env, jobject 
> > > obj);
> > > jboolean gnu_java_awt_peer_gtk_GtkImage_isOffScreen(JNIEnv *env, jobject 
> > > obj);
> > 
> > Typically we declare functions used by multiple peers in gtkpeer.h.  Is
> > it necessary to use these long JNI-style names?
> 
> Ah. Well, these are only used in 2 places, but I can move them, sure.
> As for the long style names, it's not necessary of course, I just wanted
> to follow the convention. Maybe it should be shortened to just
> GtkImage_* instead. (Including the class name seems like a good idea to
> me, since it indicates what kind of class obj is intended to be, and
> where the function is located.)

Looks like this patch broke the JNI method check. Have you tried to build
a clean tree with this patch before commiting?

Please rename the methods.


Michael
-- 
Escape the Java Trap with GNU Classpath!
http://www.gnu.org/philosophy/java-trap.html

Join the community at http://planet.classpath.org/