Re: [gentoo-dev] new old eclass - wxwidgets.eclass

[Mart Raudsepp]

Hello,

On E, 2007-09-24 at 21:14 -0600, Ryan Hill wrote:
> since everyone is getting into the reviewing mood, i thought it
> would be a good time to get some opinions on the wxwidgets
> eclass rewrite i've done.


I figured I'd reply to the old review request on the list instead of
doing this privately, so here we go.

The notes are based on the latest eclass located at
http://overlays.gentoo.org/dev/dirtyepic/browser/eclass/wxwidgets.eclass
at current revision 6:

* I just assume the documentation markup for the new way of doing eclass
documenting is good. The documentation text itself seems mostly good, if
maybe not so professional. Some nitpicking:

** Ebuilds are also required set the global -> s/required/required to/?

** The description gives the impression we support installation of
release/debug side-by-side - don't think this is true. Maybe we should
think about a way for users to be able to set their own wx-config to a
debug build, but have the system USE flag dependent. Hmm... Maybe I
should revise my stance on debug + release co-existance in some form

** Once 2.8 is in, we should update the simple usage example to that

** With EAPI=1 SLOT depends can be used now, but as wx is always with a
version and SLOT matching a way that * wildcard can be used, it's fine
to not require things to use EAPI=1 indeed I'd say. Just noting.

** In simple usage example I would rather refer to RDEPEND, not DEPEND,
or both


* Perhaps we should take care of setting the wxGTK DEPEND and RDEPEND
ourselves based on a WX_GTK_VER in ebuilds global scope? Might cause
problems for things using wx conditionally for some optional GUI.
Talking of which, it seems it already relies on global scope - does that
work fine with conditional wx usage? need-wxwidgets for that, I guess?
What's the view on eclasses touching DEPEND/RDEPEND these days, anyway?

* Taking care of DEPEND and RDEPEND in the eclass itself could allow us
to migrate things over to wxBase package in the future without any
ebuild migration work, once we fix upstream configure.in to work with a
system provided wxBase libraries and not have file collisions. However
this probably requires a global variable for base-ansi/base-unicode
passing instead of a need-wxwidgets function for that to be possible to
work out

* I hate any unconditional compiler flags workarounds, such as
"append-flags -fno-strict-aliasing". I have some long pending upstream
work to do to simply fix it in that one header where it makes noise and
once that finally gets resolved (or maybe it already is in 2.8), we have
no reason to keep passing this, as to my knowledge it also means less
optimization chances for gcc. Do we have a plan on how to get rid of
that CFLAGS append once the user is dealing with a wx library that has
this fixed?
At least this isn't the configure argument mess we used to have in
wxlib.eclass where you'd get some important class configured out just
because it was broken in an old version :)

* Are you sure we want to ewarn unconditionally on all need-wxwidgets
function calls?

* What's with the TODO on _wxerror() and the function?

The previous points encompass mostly only things noted while going over
the code - I have yet to look at it with a logic and interaction sense.
I figured I'll get this current list off my hands before I get sunk into
work.


Many many thanks for pushing wxGTK forwards while I'm occupied with
work, gnome stuff... and procrastinating

-- 
Mart Raudsepp
Gentoo Developer
Mail: leio@g.o
Weblog: http://planet.gentoo.org/developers/leio