All Bonobo patches must be approved from now on.





    I have just reverted Elliot's most recent Bonobo commit.  Patches
like this should not be applied to Bonobo.  Please make sure you have
read the HACKING file before writing code for Bonobo.

    In the future, all Bonobo patches must be approved by either
myself or Miguel before being committed.

    What was wrong with this patch:

        1. In Bonobo, we use the Gnome Programming Guidelines
           indentation and formatting style.  This patch used 2-space
           tabs and other improper formatting all over the place,
           which is not harmonious with the existing style.

        2. The EPV changes were just wrong.  First, the EPV array is
           statically initialized like this:

    	        static POA_GNOME_ProgressiveDataSink__epv pds_epv = {
		        NULL,
		        impl_start,
		        impl_end,
		        impl_add_data,
		        impl_set_size
        	};

           This is completely unmaintainable, as it requires that the
           ordering in the CORBA stubs matches the ordering in the C
           files.  Totally unacceptable.

           To make matters worse, this was done very inconsistently,
           with the epv kept in a static function variable half the
           time and put in a publicly-exported epv variable in other
           places.

           Furthermore, we've decided that it really isn't worth it to
           complicate the epv-filling functions just to save 1k of
           RAM.  It's in the noise and makes Bonobo more complicated.
           Bonobo is supposed to be the paragon of elegance and
           cleanliness, and I'm not willing to sacrifice that for a
           few bytes of savings.

        3. Elliot added ifdefs everywhere to support OAF.  How to
           support Oaf is still an open issue in Bonobo, so this was
           inappropriate.  Furthermore, ifdefs like this are ugly and
           make the code harder to maintain and read.  This will never 
           go in.  

           Just like win32 support won't go into ORBit unless it's
           done cleanly, we will not accept Oaf support in Bonobo
           unless it is done properly.

           I even objected to this patch before on this list and
           several times on IRC, and it still got applied.

        4. All Bonobo changes must have detailed ChangeLog entries.
           This patch was not such a violator in this regard (I myself 
           have been guilty of being sparse on the ChangeLogs at
           time), but there have been too many un-ChangeLogged commits 
           lately, and these have to stop.

        5. One of the *important* Bonobo interfaces
           (GnomeGenericFactory's factory method was given an extra
           argument) without any discussion on this list and without
           the corresponding updates to the programs which use
           Bonobo.


        6. The sanity check for reference counting aggregate objects
           in gnome-object.c was removed.  This is a topic which we
           have discussed many times on this list and which Miguel and 
           I have consistently objected to.  And there was no
           discussion about this change on the list before this patch; 
           just me objecting to the change!

           There is already a scheme in place in Bonobo for
           dynamically adding interfaces to aggregated objects; you
           hook up to the QI signal and return the new object with
           your handler function.  

           The GnomeObject interface functions are there to allow you
           to do simple, well-defined and easily-understandable object
           aggregation, not for tricks like this.

All of that aside, the Oaf interface seems to have some bad
aspects to it.  For example, this is not good:

#ifdef BONOBO_USE_GNOME2
	{
		char aid[160];
		g_snprintf(aid, sizeof(aid), "OAFAID:[%s,%s,%s]",
			   server->iid, server->username, server->hostname);
		
		object = add_object_to_container (aid);
	}
#else	

There needs to be a cleaner way of producing an AID.  Programmers
cannot be expected to sprinkle their code with that gross stuff.

    Finally, I should say that the changes that were made to the
exception-raising code are good and should be resubmitted for
inclusion.

    Please do not do this again.

Nat



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]