About ProgressReport and msn-pecan

Felipe Contreras felipe.contreras at gmail.com
Sun Jun 15 09:13:20 EDT 2008


On Sun, Jun 15, 2008 at 3:12 PM, Kevin Stange <kstange at pidgin.im> wrote:
> Felipe Contreras wrote:
>>
>> On Sun, Jun 15, 2008 at 1:58 AM, Tim Ringenbach
>> <tim.ringenbach at gmail.com> wrote:
>>>
>>> What's the end goal here? Just to speed up compiling and make the
>>> graph shorter, if technically misleading? (By misleading I mean the
>>> account module really does use the presence module even though it
>>> wouldn't show on the graph anymore because the header isn't including
>>> it)
>>
>> No. The public and the private fields are two completely different things.
>>
>> Try to use "hash->size" with GHashTable; you can't. Why? Because you
>> shouldn't need to know what is being used internally, actually ->size
>> is not what you would want, you need "hash->nnodes", but you don't
>> need to care, you just use g_hash_table_size.
>
> We understand this sort of design, which is the end goal of gobjectification
> and/or use of accessor functions.

Well, Tim didn't seem to understand, so I had to explain it. Can you
put that in the Wiki? I believe Sadrul would also benefit from knowing
that plan.

>> Similarly, you don't need to know about GHashNode; it's something
>> *internal*. if they choose to define that in "ghashnode.h" or
>> "ghash_private.h" shouldn't matter, because you shouldn't use that
>> directly. You might find surprising that GHashNode is not available in
>> any header file, ever. That's a clean modularization.
>
> There's no need to use the "*_private.h" headers.  If we don't want to be
> accessing data structures directly, we should just define them privately and
> use them in a standard way.  I believe that's the intention in code going
> forward.
>
>> What is _misleading_ is that you need to know all the internal details
>> of Presence; you don't, you just need the public API. Actually, I just
>> checked and Presence is properly hiding the internal details, but not
>> Account, nor Proxy which is used by Account.
>>
>
> So you see, we are already doing this.  Presence was completely rewritten
> for 2.0.0.  Account and proxy code are old and their API did not change much
> from the 0.x to 1.x to 2.x series.  It was not revamped to hide internal
> struct details, and most of the other areas have not either.  That doesn't
> mean it can't be, but hiding the structs now is an API and ABI breakage and
> cannot be done without bumping to 3.0.0.  If we want, as stated, to move to
> gobjectification, or even if we don't, this can certainly be a goal for
> 3.0.0.  Good patches would be welcome, but if we can get movement on
> gobjectification, it would be better to combine the process into one step,
> otherwise we're pushing something off to 4.0.0.
>
> However, I think the _private headers have no use at all and we can
> accomplish the same improvements without having them clutter up the source
> tree.  Nothing in other source files should need to access the members of a
> struct if the API for the objects is complete.

I agree all that private stuff needs to be gone.

But you see, my original point was that there's a header inclusion
mess, but Richard somehow doubted that, so that's why I had to
explain.

Now you still don't seem to understand that the whole point of the
_private headers is to start the struct hiding *without* API/ABI
breakage. Old stuff use foo.h (foo_private.h <- foo_public.h), new
stuff use foo_public.h.

So, internally you do all the struct hiding, but the exported headers
don't change. When everything is ready you drop foo_private.h (foo.h),
and foo_public.h becomes foo.h. Internally, libpurple doesn't need to
change anything at all because everything must be using foo_public.h
already.

I was about to rant about explaining this twice but apparently I did
it on IRC, not on the mailing list. Maybe that's the reason why you
think I'm not explaining my arguments; I'm not doing so in a visible
enough manner. So I will write a wiki article with my rants.

Best regards.

-- 
Felipe Contreras




More information about the Devel mailing list