mdn / short-descriptions

Short descriptions of web platform features, for flexible usage in applications.
Other
12 stars 6 forks source link

Lint JSON #12

Closed ddbeck closed 5 years ago

ddbeck commented 5 years ago

This PR mostly fixes #10. It adds npm run lint-json. It gives output like this (though in your terminal you'll get some highlighting):

$ npm run lint-json

> mdn-short-descriptions@0.0.1 lint-json /Users/ddbeck/Code/Mozilla/short-descriptions/ddbeck-short-descriptions
> node test/lint-json.js

descriptions/css/properties/align-content.json: First sentence may be too long. Expected ≤150; got 186
descriptions/css/properties/align-content.json: > The CSS align-content property sets how the browser distributes space between and around content items along the cross-axis of a flexbox container, and the main-axis of a grid container.
descriptions/css/properties/align-content.json: Summary is too long. Expected ≤180 characters; got 186
descriptions/css/properties/align-content.json: > The CSS align-content property sets how the browser distributes space between and around content items along the cross-axis of a flexbox container, and the main-axis of a grid container.

Checked 30 descriptions.
29 descriptions passed all checks.
1 descriptions failed one or more checks:
  descriptions/css/properties/align-content.json

Additional notes:

Let me know what you think! Thanks!

ddbeck commented 5 years ago

I didn't find the naming of sourceRules intuitive. jsonFormatRules, maybe?

Yep, that's a good idea.

since contentRules and sourceRules don't seem to share much code, and given that "lint-short-descriptions" only uses contentRules, consider splitting "linting-rules" into two files?

Yeah that makes sense. Or maybe just move sourceRules (future jsonFormatRules) to lint-json.js. I'll play with this a little.

I found the wiki attribute in contentRules a bit weird. That is, I think of it as the choice of the application (i.e. "lint-short-descriptions") which rules it wants to use. So encoding that in the rules themselves seems like the wrong place. Instead perhaps "lint-short-descriptions" could contain an array containing the descriptions of the rules it wants to use?

Also a good idea. I'll give each rule a short identifier (like eslint rules) on which to filter.

also naming, but "lint-short-descriptions" is a bit confusing now that "lint-json" is also linting short descriptions. Perhaps "lint-wiki-summaries" or something?

👍

To do list:

ddbeck commented 5 years ago

OK, fixes applied. I also changed package.json such that npm test will:

  1. Lint the JS source
  2. Run the tests for the linting rules
  3. Run the linter against the JSON