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.da...@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@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
