On Tue, 2011-01-04 at 09:40 +0100, Kamil Páral wrote:
> This patch ensures that the last remaining config file repoinfo.conf is
> also loaded from current directory by default and only when it's not
> present the system-wide location is used. It means that now all our
> config files use CWD location by default and we don't have to maintain
> /etc directory on AutoQA clients anymore.
> 
> It also adds a few comments to RepoinfoConfig object and repoinfo.conf.
> 
> Post-repo-update and post-tree-compose watchers have slightly modified
> placeholder for architecture in repo URLs.
> 
> getbool() method now does not crash for non-string inputs.

Looks good and bonus points for adding docstrings :)

I've added a few comments below, otherwise ACK from me!

> Fixes ticket #253.
> ---
>  hooks/post-repo-update/watch-repos.py     |   12 ++++++------
>  hooks/post-tree-compose/watch-composes.py |   10 +++++-----
>  lib/python/config.py                      |   16 +++++++++-------

No objections, looks like $ARCH is more self-documenting than %a.  I
wonder if we should be using string.format() instead of string.replace()
here?

http://docs.python.org/library/stdtypes.html#str.format

I'm not aware that it's better or worse than using string.replace(), I'm
just familiar with how Chris Lumens uses it for templating in the
anaconda_storage test.  Definitely your call on which is the appropriate
fit.

http://git.fedorahosted.org/git/?p=autoqa.git;a=blob;f=tests/anaconda_storage/framework/suites/__init__.py;h=382d478f3af118709d5a4bef32eafc1394d97c77;hb=refs/heads/clumens#l146

>  lib/python/repoinfo.py                    |   29 
> +++++++++++++++++++++++------

ACK, that makes sense, and kudos for the docstring additions

>  repoinfo.conf                             |    4 ++++

Good comments, thanks.

>  5 files changed, 47 insertions(+), 24 deletions(-)

Thanks,
James

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
autoqa-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/autoqa-devel

Reply via email to