[PATCH 4 of 9 V2] obsolete: add the detection of bumped changeset

Kevin Bullock kbullock+mercurial at ringworld.org
Mon Oct 15 14:27:02 CDT 2012


On Oct 15, 2012, at 7:46 AM, pierre-yves.david at logilab.fr wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at logilab.fr>
> # Date 1350295946 -7200
> # Node ID b992c8e9e2a74630a9f932e98f46a34a6252725b
> # Parent  79cf3c2a7ec3728e57f3c771187dd8cb9a92bfa1
> obsolete: add the detection of bumped changeset.
> 
> Bumped changeset are non-public changeset that tries to succeed to a public()
> changeset.
> 
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -422,10 +422,55 @@ def _computesuspendedset(repo):
> @cachefor('extinct')
> def _computeextinctset(repo):
>     """the set of obsolete parents without non obsolete descendants"""
>     return set(repo.revs('obsolete() - obsolete()::unstable()'))
> 
> + at cachefor('bumped')
> +def _computebumpedset(repo):
> +    """the set of rev trying to obsolete public revision"""
> +    # get all possible bumped changesets
> +    candidates = _allflatsuccessors(repo, repo.revs('public()'))
> +    # revision public or already obsolete don't count as bumped
> +    query = '%ld - obsolete() - public()'
> +    return set(repo.revs(query, candidates))
> +
> +def _allflatsuccessors(repo, revs):
> +    """return revid of all successors of a set of revs
> +
> +    Only know node are returned (unknown node does not have revid).  This
> +    function loose the split vs divergence information, revision are not
> +    grouped by successors set.
> +    """
> +    cl = repo.changelog
> +    pending = [cl.node(r) for r in revs]
> +    # prevent extra processing and cycle
> +    seen = set(pending)
> +    # records result
> +    successors = set()
> +    # mapping {node -> marker that use node as precursors}
> +    markersfor = repo.obsstore.precursors
> +    while pending:
> +        nc = pending.pop()
> +        # for all markers that make this node obsolete
> +        for mark in markersfor.get(nc, ()):

Since this is the only place you use nc (AFAICT), why not:

           # all markers that make this node obsolete
           markers = markersfor.get(pending.pop(), ())

...thus making the comment clutter up the code less?

> +            # for all successors

Still a useless comment.

> +            for suc in mark[1]:
> +                # record this as a successors

Still a useless comment.

> +                successors.add(suc)
> +                # add it to the list of node to proceed

This part of this comment is useless...

> +                # we want the successors of this successors too.

...whereas this part is merely a little unclear.

You may like lots and lots of comments, but Mercurial style disagrees. (Unfortunately this isn't yet documented on the CodingStyle page, hmm...)

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock



More information about the Mercurial-devel mailing list