[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