On Tue, Oct 23, 2012 at 05:05:02PM +0200, Michael Hanselmann wrote:
> 2012/10/23 Iustin Pop <[email protected]>:
> > On Tue, Oct 23, 2012 at 03:14:52PM +0200, Michael Hanselmann wrote:
> >> 2012/10/23 Iustin Pop <[email protected]>:
> >> > On Mon, Oct 22, 2012 at 06:28:34PM +0200, Michael Hanselmann wrote:
> >> >> --- a/lib/ssh.py
> >> >> +++ b/lib/ssh.py
> >> >>  def GetUserFiles(user, mkdir=False, kind=constants.SSHK_DSA,
> >> >> -                 _homedir_fn=utils.GetHomeDir):
> >> >> +                 _homedir_fn=None):
> > […]
> >> > What is this change?
> >>
> >> For unittests of prepare-node-join, where I have to mock the home
> >> directory function.
> >
> > Yes, but even before, this was passed as a (default) argument, so you
> > could override it. I'm just asking whether there is a deeper reason that
> > is not obvious to me.
> 
> Ah yes, there is. prepare_node_join.UpdateSshRoot has to forward
> “_homedir_fn”, so it easier to have a default of “None” at both
> UpdateSshRoot and ssh.GetUserFiles. The later then inserts the actual
> implementation if the keyword argument is None. Without this change,
> both functions would have to define the actual implementation as a
> default.

OK. Would you mind adding a shorter version of this to the commit
message? (e.g. "Also changed some default argument values in order to
reduce duplication").

LGTM then.

thanks,
iustin

Reply via email to