On Friday, February 06, 2015 2:10:31 AM hasufell wrote:
> Afais the eclass is missing the safeguard, as in

I wasn't aware this was needed. Basically just wrap everything except 
EXPORT_FUNCTIONS in it? I couldn't find it documented anywhere...

> Why is EAPI=5 not supported?

*Only* EAPI=5 is supported.

> Why does it not use git-r3?

I just looked into this, and I don't see a way to get git-r3 to use the same 
repositories already cloned for previous versions. What is the standard 
practice for ebuilds already using git-2?

> Are you sure all ebuilds that will use this eclass have the same
> HOMEPAGE? We had this with qt eclass once and it wasn't really correct,
> but rather a convenience shortcut.

Technically, Bitcoin Core doesn't really have any website.
For technical reasons, bitcoin.org is where we upload the release binaries.
I suppose we could point it at the GitHub URI, but that's not very user-
friendly...

> I don't understand why bitcoincore_pkg_pretend(), bitcoincore_src_test()
> are defined twice. I think you mistyped the names here slightly.

This was a remnant from an older version of the eclass, removed.

> line 169, 170 and 222 seem to lack a "|| die"

Added to 169,170 (copied libs), but 222 is expected to fail in some cases.

> autoreconf is not reliable and we know that it sometimes unpredictably
> fails. Running it unconditionally is a bit strong, no?

What should be used instead, to generate configure etc?

> bitcoincore_conf() looks like it could make use of 'usex' and
> 'use_enable' instead of those difficult to read if-else conditions.

These will give QA issues for ebuilds that don't have the applicable USE flag 
available in IUSE.

> I'm still not sure why we need to put nongeneric eclasses in eclass/ and
> cannot put them inside the ebuild dir.

Which ebuild dir? This is for 5 different packages.

Thanks for the review,

Luke

Reply via email to