[PATCH STABLE] hgweb: use introrev() for finding parents (issue4506)

Pierre-Yves David pierre-yves.david at ens-lyon.org
Fri Feb 20 10:37:04 CST 2015


On 02/19/2015 01:41 PM, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <engored at ya.ru>
> # Date 1424345526 -28800
> #      Thu Feb 19 19:32:06 2015 +0800
> # Branch stable
> # Node ID c5c1ffca678395337f8ef415423f990a9397d4ef
> # Parent  942a5a34b2d0611ab284380fbe45b9bb1897af98
> hgweb: use introrev() for finding parents (issue4506)

Overall direction is good, But I've a couple of feedback on the code.

> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -139,8 +139,8 @@ def _siblings(siblings=[], hiderev=None)
>
>   def parents(ctx, hide=None):
>       if (isinstance(ctx, context.basefilectx) and
> -        ctx.changectx().rev() != ctx.linkrev()):
> -        return _siblings([ctx._repo[ctx.linkrev()]], hide)
> +        ctx.changectx().rev() != ctx.introrev()):
> +        return _siblings([ctx._repo[ctx.introrev()]], hide)

ctx.introrev() can lead to some actual computation (in opposition to 
ctx.linkrev()). So you should call it only once and store the result 
into a temporary variable.

(This is the only "blocking" feedback)


>       return _siblings(ctx.parents(), hide)
>
>   def children(ctx, hide=None):
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -753,3 +753,42 @@ Test that removing a local tag does not
>     $ hg tags
>     visible                            0:193e9254ce7e
>     tip                                0:193e9254ce7e
> +
> +#if serve
> +
> +Test issue 4506

Small nits, we can probably use a bit more verbose title (the reference 
to the issue is good, but some actual description of the text is better).

> +  $ cd ..
> +  $ hg init repo-issue4506
> +  $ cd repo-issue4506
> +  $ echo "0" > foo
> +  $ hg add foo
> +  $ hg ci -m "0"

Other optional nits: Using bare number as commit name is a good way to 
get confused with revision number, so I advise for a something a bit 
more version (eg: "content-0")


-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list