Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format and lint Markdown text files #320

Merged
merged 9 commits into from
Feb 20, 2019
Merged

Format and lint Markdown text files #320

merged 9 commits into from
Feb 20, 2019

Conversation

jason-fox
Copy link
Contributor

@jason-fox jason-fox commented Feb 4, 2019

Similar to #314 - This change to package.json introduces markdown linting, textlint linting and markdown formatting using prettier.

  • The text is checked for spelling errors, bad grammar and dead links (on CI)
  • The markdown is checked for consistency (on CI)
  • The markdown is nicely formatted. (on Husky commit)
  • The code snippets within the markdown are formatted (on Husky commit)
npm i
npm run prettier:text
npm run lint:text
npm run lint:md

@jason-fox
Copy link
Contributor Author

Currently the Perseo Documentation has 3 dead links

./documentation/api.md
  56:67  error  https://colabora.tid.es/dca/SitePages/Inicio.aspx is dead. (307 Temporary Redirect)  no-dead-link

./documentation/architecture.md
  207:56  error  https://colabora.tid.es/dca/SitePages/Inicio.aspx is dead. (307 Temporary Redirect)                                                                       no-dead-link
  235:94  error  http://esper.codehaus.org is dead. (request to http://esper.codehaus.org failed, reason: getaddrinfo ENOTFOUND esper.codehaus.org esper.codehaus.org:80)  no-dead-link

This is the reason I commented out the Travis CI step.

package.json Outdated
@@ -39,11 +42,18 @@
"chai": "~4.1.2",
"proxyquire": "0.5.1",
"prettier": "~1.14.2",
"remark-cli": "^6.0.1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ~ in new depedencies

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 266b278 (textlint and remark)

@@ -24,7 +24,10 @@
"test": "mocha --recursive 'test/**/*.js' --reporter spec --timeout 3000 --ui bdd --exit",
"test:watch": "npm run test -- -w ./lib",
"lint": "jshint lib/ --config .jshintrc && jshint test/ --config test/.jshintrc",
"lint:md": "remark 'README.md' documentation",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three new targets should be added to https://github.com/telefonicaid/perseo-fe/blob/master/documentationT/development.md#development-documentation

In addition, what the diference between remark and textlint? Could you provide some link about it?

Copy link
Contributor Author

@jason-fox jason-fox Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The links were in the head of the PR:

  • remark-lint - check that the Markdown is well formated.
  • textlint - check for spelling mistakes, dead links, poor grammar.

remark lint

remark-lint is required because prettier can only do so much with random human markup particularly if you've mixed up * and - for bullets and haven't spaced the bullets properly or used inconsistent bolding.

How can you tell how this should really be rendered?

* **XXXXX*- YYYY

remark-lint has the same role for markdown as eslint does for JavaScript - it means you can be confident that the prettier result is good.

text lint

This does nothing to the markup - it merely checks for consistent text and links so words like FIWARE or IoT Agent can be enforced to have consistency (which is good for documentation). It also adds a link checker so the docs shouldn't contain any dead links.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Targets added to documentation - fixed b4923d7

@@ -70,6 +80,10 @@
"*.js": [
"prettier --config .prettierrc.json --write",
"git add"
],
"*.md": [
"prettier --no-config --tab-width 4 --print-width 120 --write --prose-wrap always",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command on *.md is the one achieving "The code snippets within the markdown are formatted (on Husky commit)"? What about the other mentioned in the PR body "The markdown is nicely formatted. (on Husky commit)" ?

Copy link
Contributor Author

@jason-fox jason-fox Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command on *.md is the one achieves both.

  • It nicely formats the markdown as markdown

Something like:

indented markdown      with      excess spaces 
* item one
- item two
  * item 2a
  * item 2b

Will be changed into:

indented markdown with excess spaces 

-   item one
-   item two
    -  item 2a
    -  item 2b

Properly indented, consistent markup

  • It formats the codesnippets within the markdown according to the text highlighting indicator
Some markdown text:
```javascript
   crampedFunction(){var x; return dothis();}
``
Some markdown text:

```javascript
crampedFunction (){
    var x; 
    return dothis();
}
``

In this case the snippet is recognised as JavaScript and formatted accordingly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NTC

Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

Passing the ball to @cblanco for additional LGTM before merging.

* Relax ThereIs rule to align with AGPL Disclaimer.
@cblanco
Copy link
Contributor

cblanco commented Feb 20, 2019

LGTM

Thanks for the contribution.

@fgalan
Copy link
Member

fgalan commented Feb 20, 2019

Spin-off issue: #324

@fgalan fgalan merged commit 08117b5 into telefonicaid:master Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants