Code review comment for lp://qastaging/~ted/libindicator/loader

On Tue, 2009-11-03 at 23:15 +0000, Ted Gould wrote:

> > I see Cody agree's with me :). A function that returns void, returns
> > void - it *is* noise to add a return there, because you /cannot/ return
> > anything. I'll accept your point in a function that returns non-void
> > (and so will compilers :)).
>
> No, it's explicitly stating something that is otherwise implicit. For
> instance the return has value in a situation like this, even though no
> value is returned.
...
> You're just counting on the '}' at the end of the function putting an
> implied return there. I'm saying we should be explicit about the ending
> of a function because it's a very important thing in the function.

I can see you feel strongly about this; I'm sure it will turn up in
reviews again. So far you haven't convinced Cody or myself. Its a very
idiosyncratic coding style.

I have no objection to you landing this patch with it like that, but I
think this bears more discussion: having other folk cringe every time
they see a return at the end of a function isn't great :)

> > Would it be horrible to *signal* that this does not take a ref:
> > indicator_object_get_label_(steal|unref|noref|...) ?
>
> I'd agree, but I think with out guidance from GTK/Glib I'm not sure that
> it'd be that useful. Personally, I'd rather go with _ref on functions
> that do create refs.

Lets land this branch. Perhaps you could blog (you are on planet gnome,
yes?) about this and see if there is any major feelings from the
community?

Adding _ref works fine for me too - I don't care about the specific
representation of the signal, just the presence.

-Rob