D937: remotenames: move function to pull remotenames from the remoterepo to core

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


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

INLINE COMMENTS

> remotenames.py:10
> +
> +def pullremotenames(localrepo, remoterepo, remotepath):
> +    """ pulls bookmarks and branches information of the remote repo during a

It might be better to call this `fetchremotenames` or similar since it might be called both on push and on pull

> remotenames.py:11-16
> +    """ pulls bookmarks and branches information of the remote repo during a
> +    pull or clone operation.
> +    localrepo is our local repository
> +    remoterepo is the repo from which we are pulling or cloning
> +    remotepath is the path of the remote repository
> +    """

nit: for multiline docblocks, I prefer to leave the first line empty:

  """
  pulls bookmarks and branches information of the remote repo during a
  pull or clone operation.
  
  localrepo is our local repository
  remoterepo is the repo from which we are pulling or cloning
  remotepath is the path of the remote repository
  """

> remotenames.py:22-28
> +    bmap = {}
> +    repo = localrepo.unfiltered()
> +    for branch, nodes in remoterepo.branchmap().iteritems():
> +        bmap[branch] = []
> +        for node in nodes:
> +            if node in repo and not repo[node].obsolete():
> +                bmap[branch].append(node)

It might be worth refactoring this function into:

(1) `fetchremotenames()`, a top-level function that loops through each supported namespace and then calls the appropriate fetching function for each name.
(2) `fetchremotebookmarks`, which does the appropriate stuff for bookmarks
(3) `fetchremotebranchheads`, which does the appropriate stuff for branch heads

That should make what's going on more clear.

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list