[PATCH 4 of 4] config: set a 'source' in most cases where config don't come from file but code
Matt Mackall
mpm at selenic.com
Wed Mar 19 12:44:40 CDT 2014
On Wed, 2014-03-19 at 12:25 +0100, Mads Kiilerich wrote:
> On 03/19/2014 03:56 AM, Siddharth Agarwal wrote:
> > On 03/18/2014 06:45 PM, Mads Kiilerich wrote:
> >> # HG changeset patch
> >> # User Mads Kiilerich <madski at unity3d.com>
> >> # Date 1395193514 -3600
> >> # Wed Mar 19 02:45:14 2014 +0100
> >> # Node ID e036b9c6118260654fcd567eb5653ec45bcc86a0
> >> # Parent aecee5b4ae61ae98a138583c63ce03a9e82084f0
> >> config: set a 'source' in most cases where config don't come from
> >> file but code
> >>
> >> Some extensions set configuration settings that showed up in 'hg
> >> showconfig
> >> --debug' with 'none' as source. That was confusing.
> >>
> >> Instead, they will now tell which extension they come from.
> >
> > Most of these changes won't actually be seen by hg showconfig, would
> > they? Since they only happen temporarily and are meant for certain
> > code paths.
>
> Correct. I opted for consistency instead of trying to figure which are
> relevant for common code paths.
>
> When one part of the code temporarily sets a config value that another
> read then it is quite similar to using global variables. They can be
> hard tot race. It can be convenient for debugging to have some trace of
> where that value is coming form.
>
> The cases I care about the most has been addressed in the previous
> patches ... but I think this one also adds some value. Also, the cost is
> pretty low.
>
> > In fact, for temporary config changes it would even be preferable to
> > not set the source, especially because of patch 3.
>
> Why? Some part of the code _is_ setting a config value. The value do
> thus have a source and it would be nice to know what the source is.
>
> The most important feature of patch 3 is that fixconfig preserves the
> existing source. I consider patch 3 and 4 pretty much unrelated.
>
> > Also, it really seems like this patch should be broken up into
> > multiple ones.
>
> The patch do one thing. I do not think it would add any value to split
> it up - that would just make it harder to review and discuss.
I'm ok with this. The patch has lots of very simple, very similar
changes. I can look at each one and judge that it does indeed match the
patch description in a couple seconds without scrolling back and forth
and straining my brain with a bunch of bookkeeping or
reverse-engineering.
I've gone ahead and queued these, thanks.
--
Mathematics is the supreme nostalgia of our time.
More information about the Mercurial-devel
mailing list