[PATCH 02 of 10] scmutil: delegate mustaudit property to the real opener
mads at kiilerich.com
Thu Sep 13 19:25:13 CDT 2012
Bryan O'Sullivan wrote, On 09/14/2012 01:21 AM:
> On Thu, Sep 13, 2012 at 2:56 PM, Mads Kiilerich <mads at kiilerich.com
> <mailto:mads at kiilerich.com>> wrote:
> This - and the previous change - seems a bit strange without a
> comment that the mustaudit property so far is unused.
> Isn't that a bit redundant, given what a small patch this is?
> Genuinely curious, not being argumentative.
Somewhat redundant, yes. I only found it worth saying in the passing
before making a less redundant comment.
But not completely redundant, IMO.
The patch - both the description alone and when including the diff -
answers the 'what' question but not the 'why' question. Not knowing the
intention makes it harder to review. Why is mustaudit delegated? Why do
we need this change? Is it a bug fix where it should be reviewed whether
the bug really has been fixed ... and where there probably should have
been a test? Or is it a refactoring ... where it is strange that it
doesn't have any use? Or is it an optimization?
Knowing the answer - that it is a partial refactoring which introduce
some dead 'commando pattern' code that will be used later on - makes it
a lot easier to understand and review now and later on when digging history.
More information about the Mercurial-devel