[PATCH 3 of 5] store: rename arguments/variables related with "opener" "vfs" or so

[Adrian Buehlmann]

On 2012-08-13 08:44, FUJIWARA Katsunori wrote:
> 
> At Sun, 12 Aug 2012 11:25:15 +0200,
> Adrian Buehlmann wrote:
>>
>> On 2012-08-11 14:14, FUJIWARA Katsunori wrote:
>>> # HG changeset patch
>>> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
>>> # Date 1344687075 -32400
>>> # Node ID a471147e47cf6477af91cad0f93d27340997c914
>>> # Parent  592ebd0c4ad94c3d20c5e7e4a030b84cb854bb45
>>> store: rename arguments/variables related with "opener" "vfs" or so
>>                                                          to
> 
> Once I also added "to" as you pointed. But according to my
> dictionaries, "rename" has "rename SRC DST" syntax, so I removed it.
> 
> Thank you for your comment, I'll use "to" for "rename" with confidence
> in re-posting :-)
> 
>>> diff -r 592ebd0c4ad9 -r a471147e47cf mercurial/store.py
>>> --- a/mercurial/store.py	Sat Aug 11 21:11:15 2012 +0900
>>> +++ b/mercurial/store.py	Sat Aug 11 21:11:15 2012 +0900
>>> @@ -237,12 +237,12 @@
>>>  
>>>  class basicstore(object):
>>>      '''base class for local repository stores'''
>>> -    def __init__(self, path, openertype):
>>> +    def __init__(self, path, vfstype):
>>>          self.path = path
>>>          self.createmode = _calcmode(path)
>>> -        op = openertype(self.path)
>>> -        op.createmode = self.createmode
>>> -        self.opener = scmutil.filteropener(op, encodedir)
>>> +        vfs = vfstype(self.path)
>>> +        vfs.createmode = self.createmode
>>> +        self.opener = scmutil.filteropener(vfs, encodedir)
>>>  
>>>      def join(self, f):
>>>          return self.path + '/' + encodedir(f)
>>> @@ -287,12 +287,12 @@
>>>          pass
>>>  
>>>  class encodedstore(basicstore):
>>> -    def __init__(self, path, openertype):
>>> +    def __init__(self, path, vfstype):
>>>          self.path = path + '/store'
>>>          self.createmode = _calcmode(self.path)
>>> -        op = openertype(self.path)
>>> -        op.createmode = self.createmode
>>> -        self.opener = scmutil.filteropener(op, encodefilename)
>>> +        vfs = vfstype(self.path)
>>> +        vfs.createmode = self.createmode
>>> +        self.opener = scmutil.filteropener(vfs, encodefilename)
>>>  
>>>      def datafiles(self):
>>>          for a, b, size in self._walk('data', True):
>>> @@ -381,15 +381,15 @@
>>>  _fncacheopener = _fncachevfs
>>>  
>>>  class fncachestore(basicstore):
>>> -    def __init__(self, path, openertype, encode):
>>> +    def __init__(self, path, vfstype, encode):
>>>          self.encode = encode
>>>          self.path = path + '/store'
>>>          self.createmode = _calcmode(self.path)
>>> -        op = openertype(self.path)
>>> -        op.createmode = self.createmode
>>> -        fnc = fncache(op)
>>> +        vfs = vfstype(self.path)
>>> +        vfs.createmode = self.createmode
>>> +        fnc = fncache(vfs)
>>>          self.fncache = fnc
>>> -        self.opener = _fncacheopener(op, fnc, encode)
>>> +        self.opener = _fncacheopener(vfs, fnc, encode)
>>>  
>>>      def join(self, f):
>>>          return self.path + '/' + self.encode(f)
>>> @@ -422,11 +422,11 @@
>>>      def write(self):
>>>          self.fncache.write()
>>>  
>>> -def store(requirements, path, openertype):
>>> +def store(requirements, path, vfstype):
>>>      if 'store' in requirements:
>>>          if 'fncache' in requirements:
>>>              auxencode = lambda f: _auxencode(f, 'dotencode' in requirements)
>>>              encode = lambda f: _hybridencode(f, auxencode)
>>> -            return fncachestore(path, openertype, encode)
>>> -        return encodedstore(path, openertype)
>>> -    return basicstore(path, openertype)
>>> +            return fncachestore(path, vfstype, encode)
>>> +        return encodedstore(path, vfstype)
>>> +    return basicstore(path, vfstype)
>>
>> Perhaps this would have been simpler to review if there would have been
>> one variable rename per patch (instead of doing multiple renames in the
>> same patch).
>>
>> I don't know what Matt wants here though.
> 
> In fact, I wavered between renaming them at once (like this patch) and
> renaming each below in separated patches.
> 
>   - openertype => vfstype
>   - op => vfs
> 
> Then, I chose the former, because of just renaming, not so many, and
> all of them are categorized as "var/arg related initialization of
> stores".
> 
> Of course, I'll re-post separated one , if this fixing is not suitable
> for Mercurial development style.

Personally, I would split this patch up into one variable rename per
patch. IMHO, it doesn't really matter that much how many patches this
ends producing. The patches will be very small and thus quickly reviewable.

If you rename two variables in one patch, then you combine two unrelated
changes into one patch.

See also
http://mercurial.selenic.com/wiki/OneChangePerPatch#Reviewability