[PATCH 4 of 4] config: set a 'source' in most cases where config don't come from file but code

Mads Kiilerich mads at kiilerich.com
Wed Mar 19 06:25:13 CDT 2014


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.

But yes, there is some prior art in splitting patches up in smallest 
possible pieces. I can split it up in one patch per setting, source or 
file if that is the deal breaker.

/Mads



More information about the Mercurial-devel mailing list