[PATCH 1 of 4 mergedriver] debugmergestate: explain why we create mergestate objects directly

Martin von Zweigbergk martinvonz at google.com
Wed Nov 18 12:38:54 CST 2015


On Wed, Nov 18, 2015 at 10:07 AM Siddharth Agarwal <sid at less-broken.com>
wrote:

> On 11/17/15 22:56, Martin von Zweigbergk wrote:
> > This explains why it's okay to do it, but why is it not okay to call
> > read() instead? I guess read() is the alternative here that you want
> > to avoid. The comment in the patch body explains even less about the
> > reason for avoiding it. I don't mind fixing up in flight if you
> > provide a text
>
> Yeah, sorry, meant to add that before sending but forgot.
>
> How about this for text?
>

Sounds good. I also updated the code comment to explain why read() would be
bad.


>
> We would normally use the read() constructur, but in this case it's fine
> because
> - we implement our own reading layer, so the extra parsing done by
>    read() is unnecessary
> - read() can throw an exception for unsupported merge state records,
>    but here we'd like to handle that separately
> - debugmergestate needs to be privy to mergestate internals anyway
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151118/34f5a974/attachment.html>


More information about the Mercurial-devel mailing list