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
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to