[PATCH 04 of 10] branchmap: add the tiprev (cache key) on the branchmap object

Augie Fackler raf at durin42.com
Sun Dec 23 19:45:22 CST 2012


On Dec 23, 2012, at 8:42 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:

> 
> On 24 déc. 2012, at 02:33, Augie Fackler wrote:
> 
>> On Dec 23, 2012, at 8:32 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>> On 24 déc. 2012, at 02:21, Augie Fackler wrote:
>>>> It feels strange to me to have both tipnode and tiprev on this object, partially because it feels more likely that they'll fall out of sync somehow. Then again, I suppose if that happens then we want the cache to be invalidated anyway?
>>> 
>>> They are both part of the key so we better have both of them here.
>>> 
>>> Patch 8 removes any all updates mades outside the class lowering the chance for them to go out of sync by mistake
>>> 
>>> Patch 10 makes a heavy use of both to check validity of a cache again a repo state. In particular, we read and write both on disk.
>> 
>> Is there any reason code outside of the branchmap class should ever be peeking at these attributes?
> 
> The updatecache function in branchmap still do.
> 
>> Could I persuade you to make them private in a followup?
> 
> We could probably clean up the updatecache function and make them private but:
> 
> (1) That won't happen before I'm done torturing the branchmap module for filtering purpose

Fair enough. Can you ping me when you've applied the English edits to this series and I'll pull it? It looks fine to me.

> (2) Your low patch pushing karma is harming your persuasion skill ;-)
> 
> -- 
> Pierre-Yves



More information about the Mercurial-devel mailing list