Code review comment for lp://staging/~ricardokirkner/lalita/trunk-easy_install

Revision history for this message
Facundo Batista (facundo) wrote :

Hi!

This looks very nice! One issue, though (see question #3).

A couple of questions....

1) It's ok that the config file is not "hardcoded", but given as an argument. But, if it is mandatory, why pass it like a parameter? I mean, why have a "-c" for it, instead of giving it as a first arg?

Instead of what this patch says as mandatory...

    ircbot.py [bunch of optional args] -c config_file server1 [server2 [...]]

... I propose ...

    ircbot.py [bunch of optional args] config_file server1 [server2 [...]]

(so, no "-c" for the config file).

What do you think?

2) I did "python setup.py build", and "sudo python setup.py install"... it downloaded the Twisted library again in this process... is it always needed or it should identify that I have that library already installed?

3) Finally, the setup process failed with (full log attached):

    error: Setup script exited with error: command 'gcc' failed with exit status 1

Did I do something wrong? Are the steps somewhat different?

(btw, this is why the "need fixing" in the review)

4) If I want to generate just the .egg, not install the program, what should I do?

Thanks for everything, this is good work! Hope that small issue is easily fixed, so we can have this in trunk!

review: Needs Fixing

« Back to merge proposal