This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5105

style: drop requirement to only use single lines between top-level objects
ClosedPublic

Authored by mjpieters on Oct 14 2018, 9:41 AM.

Details

Reviewers
durin42
indygreg
Group Reviewers
hg-reviewers
Commits
rHG47084b5ffd80: style: drop requirement to only use single lines between top-level objects

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mjpieters created this revision.Oct 14 2018, 9:41 AM
durin42 accepted this revision as: durin42.Oct 14 2018, 11:35 AM
durin42 added a subscriber: durin42.

I'm in favor, but want to see a broader set of thoughts on this.

indygreg accepted this revision.Oct 16 2018, 3:52 AM

At the 4.8 sprint, we agreed to repo mass rewriting with style that is more PEP-8 like. The tentative plan is to use black, which is very opinionated and doesn't give us much room for style flexibility. But regardless if we do that, I distinctly remember us saying we wanted to adopt double newline between module elements, allowing _ in names, and using CamelCase for class names. The former would be added automatically by black. The latter 2 would lead to inconsistent style in the long term, which we're OK with.

Since we agreed to allow double newlines and this patch formalizes that policy decision, I'm taking it. (I would also accept patches that removes the bit that complains about new symbols using _ and uppercase in names.)

This revision is now accepted and ready to land.Oct 16 2018, 3:52 AM
Closed by commit rHG47084b5ffd80: style: drop requirement to only use single lines between top-level objects (authored by mjpieters). · Explain WhyOct 16 2018, 3:55 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mcontrib/check-commit (2 lines)

Diff 12134

contrib/check-commit

(commitheader + r"(?!merge with )[^#]\S+[^:] ", (commitheader + r"(?!merge with )[^#]\S+[^:] ",
"summary line doesn't start with 'topic: '"), "summary line doesn't start with 'topic: '"),
(afterheader + r"[A-Z][a-z]\S+", "don't capitalize summary lines"), (afterheader + r"[A-Z][a-z]\S+", "don't capitalize summary lines"),
(afterheader + r"[^\n]*: *[A-Z][a-z]\S+", "don't capitalize summary lines"), (afterheader + r"[^\n]*: *[A-Z][a-z]\S+", "don't capitalize summary lines"),
(afterheader + r"\S*[^A-Za-z0-9-_]\S*: ", (afterheader + r"\S*[^A-Za-z0-9-_]\S*: ",
"summary keyword should be most user-relevant one-word command or topic"), "summary keyword should be most user-relevant one-word command or topic"),
(afterheader + r".*\.\s*\n", "don't add trailing period on summary line"), (afterheader + r".*\.\s*\n", "don't add trailing period on summary line"),
(afterheader + r".{79,}", "summary line too long (limit is 78)"), (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. # Forbid "_" in function name.
# #
# We skip the check for cffi related functions. They use names mapping the # We skip the check for cffi related functions. They use names mapping the
# name of the C function. C function names may contain "_". # name of the C function. C function names may contain "_".
(r"\n\+[ \t]+def (?!cffi)[a-z]+_[a-z]", (r"\n\+[ \t]+def (?!cffi)[a-z]+_[a-z]",
"adds a function with foo_bar naming"), "adds a function with foo_bar naming"),
] ]