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

On 03/29/2011 05:06 AM, Ulrich Mueller wrote:
>>>>>> On Mon, 28 Mar 2011, Aaron W Swenson wrote:
> 
>> This module is to replace the existing one that is shipped with
>> <app-admin/eselect-postgresql-1.0.3. It is an entire rewrite.
> 
> Some remarks:
> 
> - Global scope code is a no-no in eselect modules, especially if it
>   calls external programs or accesses external files. Your module
>   will be sourced for commands like "eselect modules list" and there
>   shouldn't be any global scope code being run on such occasions.
> 
Fixed.

> - Don't use eval. It's almost always an indication that you're doing
>   something wrong.
> 
Yeah, I should have used 'find' as fewer "tricks" are needed. Wonder why
I didn't do that in the first place....

> - For tests, please use [[ ]] rather than [ ] throughout.
> 
Done.

> - "output" and "path-manipulation" libraries are always loaded, so you
>   need not (and in fact, you shouldn't) inherit them.
> 
The dev guide could be clearer on that. Removed them from the inherit line.

> - Don't hardcode terminal specific escape sequences (like "\033[1m").
>   Use the highlighting functions of the output library instead (which
>   in turn use tput).
> 
Except $(highlight ...) doesn't work in plain echo. So, I just stripped
it out.

> - Suppressing stdout of commands like "do_action env update" is fine,
>   but why suppress stderr?
> 
Because 'ls' would complain that files didn't exist, such as lib*.dylib
when on a Linux system. It doesn't matter. But, using 'find' avoids this
mess.

> - Please avoid lines wider than 79 positions.
> 
Did where I could.

> See also eselect's README and the developer guide:
> <http://sources.gentoo.org/cgi-bin/viewvc.cgi/eselect/trunk/README>
Never knew that existed. Thanks for pointing that out.

> <http://www.gentoo.org/proj/en/eselect/dev-guide.xml>
> 
That's where I got all of my information.

> Ulrich
> 

Thank you for the feedback. It was quite helpful.

Now, I'm a bit sketchy on the handling of choosing which bitness for the
executables. (psql and friends.)

Sincerely,
  Mr. Aaron W. Swenson
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iF4EAREIAAYFAk2SSX0ACgkQCOhwUhu5AEk4xQD9E6jabiGSNYycJSU871p82H1O
0iIU1kBV2HMhfJId/NgA/0Zoq2O3Pcy9O2OcthsQb6e8ZipJZ+dpffMCaC0zOqrs
=L/aO
-----END PGP SIGNATURE-----

Reply via email to