[PATCH 1 of 7] tags: extract fnode retrieval in its own function
Ryan McElroy
rm at fb.com
Tue Mar 28 06:19:33 EDT 2017
Overall series mostly looks good to me, but there are enough grammar
changes and a few questions to be addressed that I think a v2 would be
worth it. I'll mark as changes requested in patchwork for now.
On 3/28/17 10:51 AM, Ryan McElroy wrote:
> On 3/28/17 7:16 AM, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>> # Date 1490673691 -7200
>> # Tue Mar 28 06:01:31 2017 +0200
>> # Node ID 1e77e505e7bacf59d1200714dc92e827523d7301
>> # Parent e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9
>> # EXP-Topic tags
>> tags: extract fnode retrieval in its own function
>>
>> My main goal here is to be able to reuse this logic easily. As a side
>> effect
>> this important logic is now insulated and the code is clearer.
>>
>> diff --git a/mercurial/tags.py b/mercurial/tags.py
>> --- a/mercurial/tags.py
>> +++ b/mercurial/tags.py
>> @@ -341,15 +341,27 @@ def _readtagcache(ui, repo):
>> # potentially expensive search.
>> return ([], {}, valid, None, True)
>> - starttime = util.timer()
>> # Now we have to lookup the .hgtags filenode for every new head.
>> # This is the most expensive part of finding tags, so performance
>> # depends primarily on the size of newheads. Worst case: no cache
>> # file, so newheads == repoheads.
>> + cachefnode = _getfnodes(ui, repo, repoheads)
>> +
>> + # Caller has to iterate over all heads, but can use the
>> filenodes in
>> + # cachefnode to get to each .hgtags revision quickly.
>> + return (repoheads, cachefnode, valid, None, True)
>> +
>> +def _getfnodes(ui, repo, nodes):
>> + """return fnode for .hgtags in a list of node
>
> This comment doesn't quite parse for me. Maybe it should be "return
> list of .hgtags fnodes for tags computation"
>
>> +
>> + return value is a {node: fnode} mapping. their will be no entry
>> for node
>
> s/their/there
> s/node/nodes (at end of line)
>
> Capitalization nitpicks: Return and There should be capitalized I think.
>
>> + without a '.hgtags' file.
>> + """
>> + starttime = util.timer()
>> fnodescache = hgtagsfnodescache(repo.unfiltered())
>> cachefnode = {}
>> - for head in reversed(repoheads):
>> + for head in reversed(nodes):
>> fnode = fnodescache.getfnode(head)
>> if fnode != nullid:
>> cachefnode[head] = fnode
>> @@ -361,10 +373,7 @@ def _readtagcache(ui, repo):
>> '%d/%d cache hits/lookups in %0.4f '
>> 'seconds\n',
>> fnodescache.hitcount, fnodescache.lookupcount, duration)
>> -
>> - # Caller has to iterate over all heads, but can use the
>> filenodes in
>> - # cachefnode to get to each .hgtags revision quickly.
>> - return (repoheads, cachefnode, valid, None, True)
>> + return cachefnode
>> def _writetagcache(ui, repo, valid, cachetags):
>> filename = _filename(repo)
>>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=kqVn49crFvd30D__LN7H59AJI5PDtBc_OGhiStGsD0U&s=9YhAyi6V_q5F8YZCOQnIl1yhJlH-jL6EuHDuFfl4-7Y&e=
More information about the Mercurial-devel
mailing list