parser: Use #-space-% to allow writing PEP8-compliant Python code#1287
parser: Use #-space-% to allow writing PEP8-compliant Python code#1287wenzeslaus merged 4 commits intoOSGeo:masterfrom
Conversation
|
Review by commits. Most relevant is 9cfc7c0. |
|
Notably, the first commit in the series works as is without the other three. The first three are reported as failed only because Travis is set to end if there is a new commit on the branch. |
ninsbl
left a comment
ninsbl
left a comment
There was a problem hiding this comment.
Sound like a reasonable change to me. Good to able to activate all (or at least more) flake8 checks!
Given the number of files changed files, I did not go through all of them, but I could not find any issues in the visible I looked at (pluss some additional where i loaded the diff) and tests are passing. Neither did I find any cases where # % has been in the python code base before, so the change should be safe.
In the light of the extent of changes, maybe pop a note to grass-dev list and point to this PR? If no one objects within two days you merge?
Accept '# %' in parser script header definition because '#%' is against PEP8 (each line of a block comment starts with a # and a single space). '#%' is still supported and documented as backwards compatibility. All documentation and generated code uses the new '# %'.
wenzeslaus
force-pushed
the
parser-pep8-comment
branch
from
410954e to
b134694
Compare
|
I'd like to backport this great PR, any objections? Needed in terms of compatibility to write "modern-style" addons... |
Waiting for backport of OSGeo/grass#1287
|
I guess one could consider being forced to break basic PEP8 rules a bug. Since both core and addon modules work, I think the change matured enough. However, backporting just the g.parser part ( |
|
Yes, the backport of just the g.parser part would be nice. |
Accept '# %' in parser script header definition because '#%' is against PEP8 (each line of a block comment starts with a # and a single space). Only minimal changes to achive this support are included. '#%' is still supported fully supported and remains in documentation. The full switch to '# %' is left for v8. Partial backport of 6352241 (OSGeo#1287).
Done in #1446. Please, let me know there if that's what you had in mind. |
Yes, exactly that - thanks. |
…1446) Accept '# %' in parser script header definition because '#%' is against PEP8 (each line of a block comment starts with a # and a single space). Only minimal changes to achieve this support are included. '#%' is still supported fully supported and remains in documentation. The full switch to '# %' is left for v8. Partial backport of 6352241 (#1287).
…Geo#1287) Accept '# %' in parser script header definition because '#%' is against PEP8 (each line of a block comment starts with a # and a single space). '#%' is still supported and documented as backwards compatibility. All documentation and generated code uses the new '# %'. Changes '#%' to '# %' for modules in scripts, gui, and temporal.
|
@wenzeslaus I'm writing a new module and it's honestly pain to have to type all these extra spaces when defining options and flags because I need to type #-space-%-space-key:-space-name a lot. I tried |
Let's have this discussion on grass-dev. Perhaps you can continue this thread: [GRASS-dev] Python and script header definitions for modules
I think it leaves comments which look too complicated, i.e., could mean be else than a plain sentence, which is correct, just not desired here. (Well, that's at least my guess just based on experience.)
I suggest vi, that's one less character to type. ;-) An old joke I guess... |
Technical correctness? In the end, we're forced to type one more space, so I didn't mind the extra "m", ;-) |
* parser: Use #-space-% to allow writing PEP8-compliant Python code Change `#%` in parser script header definition to `# %` because `#%` is against PEP8 (each line of a block comment starts with a `#` and a single space). This PR follows the changes in OSGeo/grass#1287
…Geo#693) * parser: Use #-space-% to allow writing PEP8-compliant Python code Change `#%` in parser script header definition to `# %` because `#%` is against PEP8 (each line of a block comment starts with a `#` and a single space). This PR follows the changes in OSGeo/grass#1287
…Geo#1287) Accept '# %' in parser script header definition because '#%' is against PEP8 (each line of a block comment starts with a # and a single space). '#%' is still supported and documented as backwards compatibility. All documentation and generated code uses the new '# %'. Changes '#%' to '# %' for modules in scripts, gui, and temporal.
…Geo#1287) Accept '# %' in parser script header definition because '#%' is against PEP8 (each line of a block comment starts with a # and a single space). '#%' is still supported and documented as backwards compatibility. All documentation and generated code uses the new '# %'. Changes '#%' to '# %' for modules in scripts, gui, and temporal.
5 participants
Accept '# %' in parser script header definition because '#%' is against PEP8
(each line of a block comment starts with a # and a single space).
'#%' is still supported and documented as backwards compatibility.
All documentation and generated code uses the new '# %'.