D941: remotenames: add a function to return the vfs object

ryanmce (Ryan McElroy) phabricator at mercurial-scm.org
Thu Oct 12 07:43:48 EDT 2017


ryanmce requested changes to this revision.
ryanmce added inline comments.

INLINE COMMENTS

> remotenames.py:31
>  
> +def getremotevfs(repo):
> +    """ returns a vfs object for .hg/remotenames/

This is not a great name. It implies it's a vfs on a remote peer, which is definitely not what we want to imply.
We might just call it `getvfs` in this case -- it's already in the remotenames module so it's obvious what it is if it's used outside of the module.

After reading the description, I see that this is for shared usage in the future. You should add a comment here in the code explaining that.
Also, a better way to deal with that is to build a shared vfs in core -- but that's outside of the scope of this change.

> remotenames.py:32-33
> +def getremotevfs(repo):
> +    """ returns a vfs object for .hg/remotenames/
> +    """
> +    return vfsmod.vfs(repo.vfs.join(remotenamedir))

nit: For single-line docblocks, please close the docblock on the same line:

  """returns a vfs object for .hg/remotenames/"""

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D941

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: ryanmce, mercurial-devel


More information about the Mercurial-devel mailing list