On Tue, Jan 10, 2017 at 7:42 AM, Pierre-Yves David <pierre-yves.da...@ens-lyon.org> wrote: > > > On 01/02/2017 11:27 PM, Kevin Bullock wrote: >> >> A few overall notes on this thread, and then some specific clarifications >> below: >> >> First, it is not the current project policy to defer any decisions until >> October 2017. It is also true that none of the core team is advocating >> radical changes to the way we develop Mercurial, either before or after that >> date. >> >> Second, this topic is a matter of project policy, so we should take it up >> as the steering committee if we want to take any further action on it. > > > To me, that part, and a large share of Ryan emails, highlights the fact that > we need to spend more time at the steering committee level to define our > decision process on multiple aspect of the project. For now, I would rather > see energy put in these definition first, before we start discussing more > practical aspect of the project as the one here. > My general feeling is that we have been putting the card before the hoax > here. > >>> On Dec 28, 2016, at 10:17, Pierre-Yves David >>> <pierre-yves.da...@ens-lyon.org> wrote: >>> >>> On 12/15/2016 11:38 PM, Augie Fackler wrote: >>>> >>>> (I’m trying to be brief here - hopefully it doesn’t come across as >>>> upset, because I’m not - mostly I was blindsided by a policy claim that I >>>> don’t remember.) >>>> >>>>> On Dec 15, 2016, at 3:40 PM, Pierre-Yves David >>>>> <pierre-yves.da...@ens-lyon.org> wrote: >>>>> >>>>> So, to sum up, my stance is "Let us not revisit decision made by Matt >>>>> for a while". Of course they might be case were we could make an exception >>>>> if it is really worth it. Code style does not seems to be high profile >>>>> enough for that (and is a lot of work to adjust). >>>> >>>> >>>> In broad strokes, I agree that we should make an effort to not cause a >>>> wave of sea change. My perception of this particular issue is that: >>>> >>>> 1) The codebase is already somewhat inconsistent (despite the efforts of >>>> mpm and others) when it comes to constant naming >>> >>> >>> My perception (and apparently Kevin's too) is that the codebase is >>> currently consistent¹. A very quick check of the current code seems to >>> confirm it is consistent (but that was a quick check). So, I think you need >>> to back this statement with stronger fact than just your perception. >> >> >> No, I said there is a _prevailing_ style, not that we have consistency. >> Augie was not the only one to draw the conclusion that we're not consistent >> today. There are sufficient exceptions to question the strength of the rule. >> Thank you for digging up the applicable section in CodingStyle, though. It >> seems the convention is written down after all. > > > There seems to be wording confusion here. So lets try to lift some of it. > Over the course of this thread, I said the described the code base as: > >>> consistent for the very vast majority. > > > and > >>> currently consistent¹ >>> […] >>> [1] sure there is a handful of exception. > > > This seemed compatible with your statement that we had a: > >> prevailing (though by no means universal) style > > > You seems to disagree with that compatibility, sorry for misinterpreting > you. > > > That said, the question is still open of how consistent the current > code-base is. In a previous email, I've asked for the people suggesting the > changes to provide data to justify it. But so far I've only seen two peoples > expressing feeling that the code base was too inconsistent. Feelings are > great to bootstrap questions and start leads to potential improvements. > However I don't think it is reasonable to take actions without gathering > proper data first. So far, I've not seen actual data. Only well known > anecdotal exception that don't provide serious information in themselves. > > 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. > > 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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel