-----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
>> <[email protected]> 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-----
