Update to "disallowed minor code changes"
Added section about minor typos in comments being disallowed, following the discussion at the Glance meeting on 3 November. Change-Id: Ic6c6369895a7095c01c1810358f4355c5cc8f0fa
This commit is contained in:
parent
cde9f18db1
commit
61e9858ac7
@ -18,6 +18,36 @@ scripts confuses operators and administrators -- we only want them to notice
|
|||||||
serious problems. Their preference must take precedence over fixing spell
|
serious problems. Their preference must take precedence over fixing spell
|
||||||
errors.
|
errors.
|
||||||
|
|
||||||
|
Typographical errors in comments
|
||||||
|
--------------------------------
|
||||||
|
|
||||||
|
Comments are not user-facing. Correcting minor misspellings or grammatical
|
||||||
|
errors only muddies the history of that part of the code, making ``git blame``
|
||||||
|
arguably less useful. So such changes are likely to be rejected. (This
|
||||||
|
prohibition, of course, does not apply to corrections of misleading or unclear
|
||||||
|
comments, or for example, an incorrect reference to a standards document.)
|
||||||
|
|
||||||
|
Misspellings in code
|
||||||
|
--------------------
|
||||||
|
|
||||||
|
Misspellings in function names are unlikely to be corrected for the "historical
|
||||||
|
clarity" reasons outlined above for comments. Plus, if a function is named
|
||||||
|
``mispelled()`` and a later developer tries to call ``misspelled()``, the
|
||||||
|
latter will result in a NameError when it's called, so the later developer will
|
||||||
|
know to use the incorrectly spelled function name.
|
||||||
|
|
||||||
|
Misspellings in variable names are more problematic, because if you have a
|
||||||
|
variable named ``mispelled`` and a later developer puts up a patch where an
|
||||||
|
updated value is assigned to ``misspelled``, Python won't complain. The "real"
|
||||||
|
variable won't be updated, and the patch won't have its intended effect.
|
||||||
|
Whether such a change is allowed will depend upon the age of the code, how
|
||||||
|
widely used the variable is, whether it's spelled correctly in other functions,
|
||||||
|
what the current test coverage is like, and so on. We tend to be very
|
||||||
|
conservative about making changes that could cause regressions. So whether a
|
||||||
|
patch that corrects the spelling of a variable name is accepted is a judgment
|
||||||
|
(or is that "judgement"?) call by reviewers. In proposing your patch, however,
|
||||||
|
be aware that your reviewers will have these concerns in mind.
|
||||||
|
|
||||||
Tests
|
Tests
|
||||||
-----
|
-----
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user