Constant naming convention

[Pierre-Yves David]

On 01/10/2017 06:29 PM, Martin von Zweigbergk wrote:
> On Tue, Jan 10, 2017 at 7:42 AM, Pierre-Yves David
> <pierre-yves.david at ens-lyon.org> wrote:
>> […]
>> So, I went to gather basic data about the actual situation. I used basic
>> grepping of assignment without any indentation in mercurial/ and hgext/
>> pythons file. This isn't perfect but that gives some basic data. I've
>> filtered out vendor-ed code, many of the re-assignment from other modules
>> and code directly tied to a specific OS API. Assignment of both type can
>> contains '_', (especially leading '_'). Here are my finding:
>
> For the record, if you include the vendored code (mostly
> mercurial/win32.py, mercurial/httpclient/, hgext/fsmonitor/,
> hgext/zeroconf/), you get this:
>
> $ find mercurial/ hgext -name '*.py' | xargs grep '^[_A-Z0-9]\+ = ' |
> grep -v ' = .*\..*' | wc -l
> 136
>
> $ find mercurial/ hgext -name '*.py' | xargs grep '^[_a-z0-9]\+ = ' |
> grep -v ' = .*\..*' | wc -l
> 436
>
> I think it's fair to include this data so readers can decide for
> themselves whether to include it or not. It's part of our code base,
> after all.

I really don't think vendored code are relevant when talking about style 
consistency in our own code base. Vendored package are alien code we 
dropped in our tree and never edit directly. We just includes them in 
our tree for packaging convenience. Vendored code follows its own code 
style defined by upstream and is explicitly excluded from our own style 
checkers. So they are definitely outside our scope when it comes to code 
style decision.

(I'm not opposed to having these number somewhere, but I've strong 
feeling them  being irrelevant when we talk about style in the Mercurial 
code base.)

>> upper case assignment:
>> ----------------------
>>
>> Only 4 modules or packages use "many" upper case constants. 3 is core, 1 in
>> hgext (various parts of 'convert'). Adding the one with 1 or two instances,
>> we got up to 10 modules/package with upper case constants, (about 40
>> assignments total). Most of it is very old code (probably prior Matt started
>> enforcing current style) and a few are recent slip in style-review.
>>
>> Detailed data below:
>>
>>   mercurial/revlog.py (10 cases)
>>   mercurial/hgweb/ (9 cases)
>>   mercurial/httpconnection.py 1 case
>>   mercurial/util.py 2 case
>>   mercurial/graphmod.py (5 spots)
>>   hgext/factotum.py 1 cases
>>   hgext/largefiles/ 1 cases
>>   hgext/highlight/ 1 cases
>>   hgext/mq.py 1 case
>>   hgext/convert/ (8 case)
>>
>> Lower case assignment:
>> ----------------------
>>
>> Full check on lower case assignments is harder because there is a large
>> quantity of them. I've filtered out thing that standed out as vendored code
>> and the vast majority of local assignments from other modules (eg 'pickle =
>> util.pickle'). The result seems mostly composed of actual constant
>> assignment for scanning through it.
>>
>> That filtering leaves about 500 instances of lower case assignments, spread
>> over 120 files.
>>
>> Conclusion
>> ----------
>>
>> Lets drop about 20% more of the lowercase assignments and files to take in
>> account the imperfect filtering. This still lives use with a 1 vs 10 ration
>> in favor of lower case for both "assignment count" and "module". This is a
>> bit higher than what I expected but still show "upper case" as rare
>> occurrence (<10%, handful of modules).
>>
>> So from my point of view, the occurrence is rare-enough and limited to
>> few-enough module to consider the currently documented style (lower for all)
>> as vastly prevailing and the code base as overall consistent.
>> (I'm of course okay with hearing other conclusion based on these data (or
>> better data)).
>>
>> It is also interesting to note that use of upper case is concentrated in
>> some key module (eg: revlog, hgweb). That might play a key part in the
>> strong contrast in consistency perception between the feeling of the two
>> people who said the code to be too inconsistent and my feeling that is was
>> overall consistent.
>>
>>
>>
>> Of course, The extraction I did is fairly basic. For example it does not
>> takes in accounts indented, but still module level constants. In addition,
>> the data about lower case assignment can be refined further. But I already
>> spent more time digging this that I was willing to, so I did not went
>> further. If someone else wants to keeps digging, feels free to do it.
>>
>>> […]
>>
>>
>> Cheers,
>>
>> --
>> Pierre-Yves David
>>
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

-- 
Pierre-Yves David