ryanmce requested changes to this revision.
ryanmce added a comment.

  I think we need to take a step back and think through how to implement this 
more robustly than we have currently done in the remotenames extension:
  
  - storage file names
  - storage format
  
  I like that you've split branches and bookmarks, but I suspect that storing 
all paths in the same file isn't the best option.

INLINE COMMENTS

> remotenames.py:35
> +
> +    `node remotepath/bookmarkname`
> +

As per the issue you raised about paths containing '/', this probably is 
insufficient.

> dlax wrote in remotenames.py:64
> Functions `saveremotebranches` and `saveremotebookmarks` are very similar, 
> only differing by the vfs file name. Perhaps there could be a single function 
> taking the filename as a parameter?

+1

> remotenames.py:67-69
> +    """ save remotenames i.e. remotebookmarks and remotebranches in their
> +    respective files under ".hg/remotenames/" directory.
> +    """

nit: start multiline docblocks on next line

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: ryanmce, dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to