Skip to content
This repository has been archived by the owner on Jun 21, 2022. It is now read-only.

bug 1354243: add macro linter (for JSON "macros") #159

Merged
merged 2 commits into from Apr 19, 2017

Conversation

escattone
Copy link
Contributor

  • expand the make lint-macros command to include the linting of JSON data files within the macros directory (using the newly added jsonlint-cli)
  • add make lint-macros to the "Test" stage of the Jenkins pipeline
  • fix JSON syntax error in macros/L10n-HTML.json so make lint-macros runs clean

* add JSON "macro" linter (jsonlint-cli)
* add lint step to "Test" stage of Jenkins
  pipelines
* fix bug in "macros/L10n-HTML.json" so
  "make lint-macros" runs clean
@escattone
Copy link
Contributor Author

escattone commented Apr 14, 2017

I wanted to also add a CSS linter (for the one CSS file within the macros directory - there are actually two files that have the .css extension, but macros/L10n-CSS.css is actually a JSON file and should probably use the .json extension instead?). I went down the path of trying stylelint, but realized I was a fish-out-of-water when it came to setting up a strictly-syntax-error-only configuration. If we think it's worthwhile, I can work later with @stephaniehobson to set that up.

@stephaniehobson
Copy link
Contributor

I did a quick look for a stylelint configuration we could borrow just to check for syntax errors but didn't turn anything up. I don't know how long it would take to put together a simple syntax check ourselves.

On the other hand, it could be argued that we should not have any CSS in this repo at all. Not even inline styles.

@jwhitlock
Copy link
Contributor

I agree:

@escattone
Copy link
Contributor Author

Thanks for the feedback @stephaniehobson and @jwhitlock !

  • renamed L10n-CSS.css to L10n-CSS.json
  • deleted CustomSampleCSS.css

Copy link
Contributor

@jwhitlock jwhitlock left a comment

Choose a reason for hiding this comment

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

Tested locally, with good JSON in PR and by adding some bad JSON and getting an error. Good fixes to JSON, and abolishing CSS from the repo. ❤️ this PR.

@jwhitlock jwhitlock merged commit a3ec33c into master Apr 19, 2017
@jwhitlock jwhitlock deleted the add-json-linter-1354243 branch April 19, 2017 14:49
jwhitlock added a commit to mdn/kuma that referenced this pull request Apr 19, 2017
- mdn/kumascript#157 - GroupData: Add Media Session API
- mdn/kumascript#158 - SpecName, Spec2:  Update HTML5.1, Add HTML5.2
- mdn/kumascript#159 - Add JSON linter, remove CSS
- mdn/kumascript#160 - GroupData: Add Cred Mgmt, Intersect Obs APIs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants