All Bonobo patches must be approved from now on.
- From: Nat Friedman <nat helixcode com>
- To: Elliot Lee <sopwith redhat com>
- Cc: gnome-components-list gnome org
- Subject: All Bonobo patches must be approved from now on.
- Date: Fri, 10 Dec 1999 00:37:47 -0500 (EST)
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]