Ok, let me know if you want me to redo the patch with the short function name.
On Fri, Feb 12, 2010 at 2:57 AM, Lucas Meneghel Rodrigues <[email protected]>wrote: > On Thu, 2010-02-11 at 23:18 -0800, K.D. Lucas wrote: > > Lucas, > > > > Thanks for your comments. I'll get a good svn diff patch out soon. > > > > Before I do though, just one question. I kind of patterned my > > get_value off of the same thing that is done in <AT_ROOT>/tko/db.py, > > so that is where I got the idea for that. I wanted to kind of shorten > > the path to this function as it would really make the code look ugly > > since the constant is quite long to begin with. > > Hmm, you are right. I guess in this case that is OK, just keep it the > way it was on your patch. > > > Also, is svn diff ok to use or would you prefer git? > > svn diff is totally OK, I just suggested git because I find it easier. > Just go ahead and do it on svn. > > > Kelly > > > > On Thu, Feb 11, 2010 at 6:31 PM, Lucas Meneghel Rodrigues > > <[email protected]> wrote: > > On Thu, 2010-02-11 at 17:44 -0800, K.D. Lucas wrote: > > > I'm currently working with a class of machines with very > > limited disk > > > space; consequently, I'd like to move the following constant > > to a > > > place where it is a little more convenient to configure. > > > > > > Hi, some comments below: > > > > > I'd like to move AUTOTEST_GB_DISKSPACE_REQUIRED = 20 to a > > value > > > derived from global_config.ini file. > > > > > > > > > --- global_config.ini (revision 4230) > > > +++ global_config.ini (working copy) > > > > > > Your mailer chewed up your patch, you'll have to find a way to > > make it > > not auto-wrap the patch contents. > > > > > [SERVER] > > > +# Minimum amount of disk space required for AutoTest in GB > > > +gb_diskspace_required: 5 > > > > > > ===================================================================== > > > and then implement the following change in > > > <AUTOTEST_ROOT>/server/hosts/abstract_ssh.py > > > > > > Point out manually where the change is is also not a good > > practice, you > > should generate the contents of the patch all at once, p1 > > level, using > > an appropriate tool such as svn diff, or better, git. > > > > If you are interested I can send you a quick start guide of > > how to > > re-create this patch using git and send it with git > > send-email. It will > > resolve all the formatting problems on your current patch. > > > > > > > > ====================================================================== > > > --- abstract_ssh.py (revision 4230) > > > +++ abstract_ssh.py (working copy) > > > @@ -5,11 +5,12 @@ > > > from autotest_lib.client.common_lib.global_config import > > > global_config > > > > > > > > > -enable_master_ssh = > > global_config.get_config_value('AUTOSERV', > > > - > > > 'enable_master_ssh', > > > - > > type=bool, > > > default=False) > > > +get_value = global_config.get_config_value > > > > > > It'd be preferable to not do the above, as we keep this import > > style to > > make it easier for unit tests, and also, get_value is a very > > generic > > name, which is a bad idea in terms of namespace. > > > > > +enable_master_ssh = get_value('AUTOSERV', > > 'enable_master_ssh', > > > type=bool, > > > + default=False) > > > > > > + > > > def make_ssh_command(user="root", port=22, opts='', > > > hosts_file='/dev/null', > > > connect_timeout=30, > > alive_interval=300): > > > base_command = ("/usr/bin/ssh -a -x %s -o > > > StrictHostKeyChecking=no " > > > @@ -487,9 +488,11 @@ > > > > > > > > > # tunable constants for the verify & repair code > > > - AUTOTEST_GB_DISKSPACE_REQUIRED = 20 > > > + AUTOTEST_GB_DISKSPACE_REQUIRED = get_value("SERVER", > > > + > > > "gb_diskspace_required", > > > + type=int, > > > + default=20) > > > > > > - > > > def verify_connectivity(self): > > > super(AbstractSSHHost, self).verify_connectivity() > > > > > > > > > The general approach looks good to me, but you have to fix the > > issues > > found and re-send so we can consider your patch for inclusion. > > > > Lucas > > > > > > > > > > > > -- > > K.D. Lucas > > [email protected] > > > -- K.D. Lucas [email protected]
_______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
