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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon created an issue. See original summary.

jhodgdon’s picture

Issue tags: +Novice
jhodgdon’s picture

Issue summary: View changes

Update: 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.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vadim.hirbu’s picture

Status: Active » Needs review
FileSize
4.61 KB

Reformatted content into README.md file.

jhodgdon’s picture

Status: Needs review » Needs work

Thanks 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.

vadim.hirbu’s picture

Updated previous patch according to comments above.

vadim.hirbu’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

Thanks 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:

module they would go in ```core/modules/action/tests/src/Nightwatch```.

Should be

module they would go in `core/modules/action/tests/src/Nightwatch`.

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.

vadim.hirbu’s picture

Status: Needs work » Needs review
FileSize
6.41 KB

Updated previous patch according to latest recommendations.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
6.4 KB

Thanks -- 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:

* Install 
  [Google Chrome](https://www.google.com/chrome/browser/desktop/index.html)

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.

vadim.hirbu’s picture

Thanks 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.

jhodgdon’s picture

I 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!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 589f610f80 to 8.8.x and fc64f3e274 to 8.7.x. Thanks!

  • alexpott committed 589f610 on 8.8.x
    Issue #3038378 by vadim.hirbu, jhodgdon: README.md in core/tests is...

  • alexpott committed fc64f3e on 8.7.x
    Issue #3038378 by vadim.hirbu, jhodgdon: README.md in core/tests is...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.