core/tests/README.md has some formatting issues.
Files with the .md extension should have good formatting both when read via Markdown processing, and in plain text (like in an 80-character text editor or command line window).
This file has two human-readable issues:
a) Some lines are longer than 80 characters. It's OK for code lines (like commands) to exceed this, but the text shouldn't.
b) Inconsistent formatting of the ``` areas. Some are left-justified, and some are indented. Probably left-justified is better.
Comment | File | Size | Author |
---|---|---|---|
#11 | 3038378-11.patch | 6.4 KB | jhodgdon |
Comments
Comment #2
jhodgdonComment #3
jhodgdonUpdate: It looks like this is the only README.md file in core, aside from ones in "vendor" directories (which we should not be editing). So, limiting this issue to just that one .md file.
Comment #5
vadim.hirbu CreditAttribution: vadim.hirbu at FFW commentedReformatted content into README.md file.
Comment #6
jhodgdonThanks for the patch!
It's not quite right though... The point of this issue is that formatting should be consistent between the sections of this file, and it should be readable on an 80-character-wide terminal. With your current patch, there are still some problems and inconsistencies:
a) In a bullet list, sometimes * is used and sometimes - is used for the bullets. Let's standardize on * (which seems to be the majority).
b) For the 2nd, 3rd, etc. lines within one bullet of a bullet list, there should always be 2 spaces of indentation. In your patch, it looks like you used either a tab (which we don't normally want in any Drupal files) or 8 spaces.
c) Sometimes commands at the command line are inline with `command here` and sometimes they are in code blocks with ```. Let's make them all ```.
d) Sometimes the ``` lines are indented from the left, and sometimes they aren't. Let's standardize on not indenting.
e) There should not be any spaces at the ends of lines. Most coding text editors have an option to remove them or to at least highlight them. There are a lot in your patch.
f) There are still a few lines that are over 80 characters.
Comment #7
vadim.hirbu CreditAttribution: vadim.hirbu at FFW commentedUpdated previous patch according to comments above.
Comment #8
vadim.hirbu CreditAttribution: vadim.hirbu at FFW commentedComment #9
jhodgdonThanks for the new patch! Much better -- a few problems still:
a) There are still spaces at the ends of quite a few lines in the Nightwatch section.
b) There are still a few lines in that section are over 80 characters, and a few spots where there is a short line but not a paragraph break. All lines should be as close to 80 characters as possible within a paragraph, without going over.
c) If code is inline, it should use ` not ```, so this line is incorrect:
Should be
And there are a few other places like that in your patch.
See https://guides.github.com/features/mastering-markdown/ for docs on Markdown formatting.
Comment #10
vadim.hirbu CreditAttribution: vadim.hirbu at FFW commentedUpdated previous patch according to latest recommendations.
Comment #11
jhodgdonThanks -- you're almost there! You really need to configure your text editor either to remove end-of-line spaces, or to highlight them in some bright color. There is still one remaining in this line:
I don't see any other problems with the latest patch.
Also, for the next time you make a core contribution... we normally make "interdiff" files when we upload new patches to issues that already had patches. You can learn about interdiffs on this documentation page: https://www.drupal.org/documentation/git/interdiff
Anyway, since it was only this 1 line that needed an update, I went ahead and made a new patch to save time. The only change is removing the space at the end of that one line. And I'll go ahead and mark it RTBC.
Comment #12
vadim.hirbu CreditAttribution: vadim.hirbu at FFW commentedThanks for review and suggestions, @jhodgdon
I'm using Phpstorm editor and It have already set config
Strip trailing spaces on Save
but this is not helping with this patch.What else config I have to set to remove white spaces at the end of line.
Comment #13
jhodgdonI don't know the settings for PHPstorm editor sorry. That sounds like the right setting. Maybe it doesn't apply to .md files? Not sure. Anyway, thanks again for the patch!
Comment #14
alexpottCommitted and pushed 589f610f80 to 8.8.x and fc64f3e274 to 8.7.x. Thanks!