Re: [gentoo-dev] New Eclasses: postgres and postgres-multi

[Ian Stakenvicius]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 23/01/16 10:51 AM, Ian Stakenvicius wrote:
>
>
>> On Jan 22, 2016, at 3:11 PM, Aaron W. Swenson
>> <titanofold@g.o> wrote:
>>
>>
>> I would like some feedback on the documentation/comments in
>> the eclass. I'm certain it could be improved. Though, if you
>> were able to follow them (not a slight, just you were the first
>> to follow them), I might have done good enough.
>>
>
> I'll have another look; i don't think I read any of the docs,
> just looked to see that PG_CONFIG was set for me and confirmed
> things looked to work the same as multilib-minimal.

OK here's my full review (with docs and all):

postgres.eclass:

postgres_check_slot() <-- #1, not seeing a need for this as
dependencies should be forced now due to USE flag provisions.  #2,
though, the documentation says it should be used in pkg_pretend but
then in the code there's an exclusion to make it a no-op when it's
run in pkg_pretend.  Perhaps it's supposed to be run in pkg_setup??

postgres_new_user() <-- the documentation seems a little bit unclear
on this one..  It looks like it creates the 'postgres' user/group
and optionally it also creates an additional specified user/group..
 But i'm not sure how creating new users/groups here instead of with
calling enewuser/enewgroup directly in the ebuild is helpful per
se...?  It also seems a bit redundant if you wanted to create say, 2
or 3 extra users/groups, to repeatedly call enewuser postgres ....


postgres-multi.eclass:

#1 - main description could perhaps be changed from:

# build against any and all compatible PostgreSQL slots that are also
# enabled by the user.

to:

# build and install for one or more PostgreSQL slots as specified by
# POSTGRES_TARGETS use flags.

#2 - POSTGRES_COMPAT , needs an example and/or text to describe the
syntax of the slot specification.

#3 - _POSTGRES_UNION_SLOTS -- you actually mean here the
intersection between the two sets, rather than the union, right?
Var can likely stay as-is but the description should probably be
updated.

#4 - postgres-multi_foreach() -- this actually runs in the builddir,
not the source dir as mentioned in the text.

#5 - postgres-multi_forbest() -- maybe expand on what the "best"
slot is here, ie that it's the lexicographically biggest slot rather
than something else (like, say, the slot that's currently eselected)

#6 - need docs for postgres-multi_src_prepare() and so forth -- just
simple stuff will suffice I think:  postgres-multi_src_prepare()
copies ${S} into a build directory for each target PG_SLOT and
should be specified; the others are default functions that are
provided for convenience, but postgres-multi_foreach should be used
instead of these when customization is necessary.

...and I think that's it..

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iF4EAREIAAYFAlamP+kACgkQAJxUfCtlWe2wAAD/QlCKmkKl/9gr42ggY9RUpvUL
HLT1trvmzvto8QLXLeoBAOWRobPqIU4fcoI0on6CJdeVS+ZEK5MFcdf9qDn3yECA
=VjiJ
-----END PGP SIGNATURE-----