Patchwork D2010: check-commit: allow foo_bar naming in functions

login
register
mail settings
Submitter phabricator
Date Feb. 2, 2018, 6:14 p.m.
Message ID <differential-rev-PHID-DREV-igqtshpvxdznplee7uko-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/27238/
State New
Headers show

Comments

phabricator - Feb. 2, 2018, 6:14 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  nameswithallthewordssmashedtogetherarehardtoread.
  especiallyifenglishisnotyourprimarylanguage.
  
  Let's align with the rest of the programming universe and
  allow_the_use_of_underscores_in_names.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2010

AFFECTED FILES
  contrib/check-code.py
  contrib/check-commit
  tests/test-contrib-check-commit.t

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 2, 2018, 6:15 p.m.
pulkit accepted this revision.
pulkit added a comment.


  I am +1 on this.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2010

To: indygreg, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - Feb. 2, 2018, 6:17 p.m.
durin42 accepted this revision as: durin42.
durin42 added a comment.


  I'm also +1 on this.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2010

To: indygreg, #hg-reviewers, pulkit, durin42
Cc: durin42, pulkit, mercurial-devel
phabricator - Feb. 3, 2018, 8:35 a.m.
yuja added a comment.


  I'm kinda -1 on this because of inconsistency with the 10-year codebase.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2010

To: indygreg, #hg-reviewers, pulkit, durin42
Cc: yuja, durin42, pulkit, mercurial-devel
phabricator - Feb. 4, 2018, 5:46 a.m.
av6 added a comment.


  As opposed to 11 words smashed together, function names in Mercurial usually are 2 or 3 words (if not just one), which is not at all bad. I think breaking inconsistency with the existing code base would be worse than making people get used to no-spaces style (the one that python's stdlib usually follows, in fact). So, FWIW, -1 from me.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2010

To: indygreg, #hg-reviewers, pulkit, durin42
Cc: av6, yuja, durin42, pulkit, mercurial-devel

Patch

diff --git a/tests/test-contrib-check-commit.t b/tests/test-contrib-check-commit.t
--- a/tests/test-contrib-check-commit.t
+++ b/tests/test-contrib-check-commit.t
@@ -132,8 +132,6 @@ 
    This has no topic and ends with a period.
   19: adds double empty line
    +
-  20: adds a function with foo_bar naming
-   + def blah_blah(x):
   23: adds double empty line
    +
   [1]
diff --git a/contrib/check-commit b/contrib/check-commit
--- a/contrib/check-commit
+++ b/contrib/check-commit
@@ -41,12 +41,6 @@ 
     (afterheader + r".{79,}", "summary line too long (limit is 78)"),
     (r"\n\+\n( |\+)\n", "adds double empty line"),
     (r"\n \n\+\n", "adds double empty line"),
-    # Forbid "_" in function name.
-    #
-    # We skip the check for cffi related functions. They use names mapping the
-    # name of the C function. C function names may contain "_".
-    (r"\n\+[ \t]+def (?!cffi)[a-z]+_[a-z]",
-     "adds a function with foo_bar naming"),
 ]
 
 word = re.compile('\S')
diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -263,8 +263,6 @@ 
     (r'.{81}', "line too long"),
     (r'[^\n]\Z', "no trailing newline"),
     (r'(\S[ \t]+|^[ \t]+)\n', "trailing whitespace"),
-#    (r'^\s+[^_ \n][^_. \n]+_[^_\n]+\s*=',
-#     "don't use underbars in identifiers"),
     (r'^\s+(self\.)?[A-Za-z][a-z0-9]+[A-Z]\w* = ',
      "don't use camelcase in identifiers", r'#.*camelcase-required'),
     (r'^\s*(if|while|def|class|except|try)\s[^[\n]*:\s*[^\\n]#\s]+',