Patchwork [1,of,5] localrepo: add some comment about role of various vfs object

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1470397745 -7200
#      Fri Aug 05 13:49:05 2016 +0200
# Node ID b34202587b00d949dd77ff97399e50d6340779a6
# Parent  3f8f53190d6afed0aca6c6527236edad28ce785c
# EXP-Topic vfs.cleanup
localrepo: add some comment about role of various vfs object

This should make things clearer for most people.

I'm a big +1 on adding documentation -- thanks for doing this!


On 3/1/17 6:58 PM, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1470397745 -7200
> #      Fri Aug 05 13:49:05 2016 +0200
> # Node ID b34202587b00d949dd77ff97399e50d6340779a6
> # Parent  3f8f53190d6afed0aca6c6527236edad28ce785c
> # EXP-Topic vfs.cleanup
> localrepo: add some comment about role of various vfs object
>
> This should make things clearer for most people.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -253,7 +253,12 @@ class localrepository(object):
>   
>       def __init__(self, baseui, path, create=False):
>           self.requirements = set()
> +        # vfs to access the working copy
>           self.wvfs = scmutil.vfs(path, expandpath=True, realpath=True)
> +        # vfs to access the content of the repository

This comment doesn't help me a lot -- it requires that I understand that 
the "repository" means "things under the .hg directory", which is 
non-obvious to newcomers to the code. I think it would be more clear to say:

# vfs to access repo files in .hg directory (excluding .hg/store)

> +        self.vfs = None
> +        # vfs to access the store part of the repository

Consider, perhaps:

# vfs to access repo history under .hg/store

> +        self.svfs = None
>           self.wopener = self.wvfs
>           self.root = self.wvfs.base
>           self.path = self.wvfs.join(".hg")
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=0Wa7QvUc5RMmEg0g8AsgL0S7_VRXgwF_-YNIWt3wfP8&s=TNGKczgSgGUR0zek-5nxBhh7u1P8sfbYWw5-do7ANyU&e=

On Thu, 2 Mar 2017 08:20:17 -0800, Ryan McElroy wrote:
> I'm a big +1 on adding documentation -- thanks for doing this!
> 
> 
> On 3/1/17 6:58 PM, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> > # Date 1470397745 -7200
> > #      Fri Aug 05 13:49:05 2016 +0200
> > # Node ID b34202587b00d949dd77ff97399e50d6340779a6
> > # Parent  3f8f53190d6afed0aca6c6527236edad28ce785c
> > # EXP-Topic vfs.cleanup
> > localrepo: add some comment about role of various vfs object
> >
> > This should make things clearer for most people.
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -253,7 +253,12 @@ class localrepository(object):
> >   
> >       def __init__(self, baseui, path, create=False):
> >           self.requirements = set()
> > +        # vfs to access the working copy
> >           self.wvfs = scmutil.vfs(path, expandpath=True, realpath=True)
> > +        # vfs to access the content of the repository
> 
> This comment doesn't help me a lot -- it requires that I understand that 
> the "repository" means "things under the .hg directory", which is 
> non-obvious to newcomers to the code. I think it would be more clear to say:
> 
> # vfs to access repo files in .hg directory (excluding .hg/store)
> 
> > +        self.vfs = None
> > +        # vfs to access the store part of the repository
> 
> Consider, perhaps:
> 
> # vfs to access repo history under .hg/store

I slightly prefer the new one, but I hesitated to update them in flight.
Can you send a follow up?

On 3/3/17 5:49 AM, Yuya Nishihara wrote:
> I slightly prefer the new one, but I hesitated to update them in flight.
> Can you send a follow up?
Will do. Since I haven't sent anything to the list in a while it will be 
good practice!