search

emberjs / guides

This repository is DEPRECATED!
https://github.com/ember-learn/guides-source
Other
283 stars 873 forks source link

Lint code snippets #613

Open locks opened 9 years ago

locks commented 9 years ago

Lint code snippets and fail the build if they don't pass.

Edit by acorncom: This is a good library to look into: https://github.com/Widdershin/markdown-doctest

michaelrkn commented 9 years ago

@rwjblue is there something in the Ember.js codebase that could be re-used here?

mtthgn commented 8 years ago

I'm interested in this issue. Here's my initial thoughts after doing a little digging

  1. It doesn't look like there's a good (and by "good", I mean maintained) jshint wrapper for ruby. I don't like the idea of having a dependency outside of the Gemfile, tho.
  2. Every jshint I looked at operated on files and directories. The ideal situation for this issue would be to operate against a string. We might have to extract every snippet, make a temp directory, and then run the linter against those temporary snippet files.
  3. Is there a canonical "Ember's .jshintrc" somewhere? Ember CLI has one, would that be the best one to use?
michaelrkn commented 8 years ago

@mtthgn Thanks for your interest in helping out with this! I don't know that I can add anything to points 1 and 2, but for 3, I'd suggest checking out https://github.com/emberjs/ember.js. My understanding is that PRs against ember.js are linted against their style guide, but I don't know where the style guide is tested against and if that code could be re-used - @rwjblue may know.

michaelrkn commented 8 years ago

Actually, regarding 1, what about https://github.com/damian/jshint?

mtthgn commented 8 years ago

@michaelrkn dunno how I didn't see that one! I'll see what I can get done on this over the weekend.

michaelrkn commented 8 years ago

:) Also, maybe instead of extracting the code snippets from our Markdown files, saving them into files, and then having the linter check them, you could fork the linter and add a way to create new input sources. Something like creating an API that can be passed a string of JavaScript to be linted, and then separating out the actually getting of the JS into another class, so that anybody can provide an arbitrary source of JS. Then turn the existing file scanning logic into one source, and create your own input source that looks through our Markdown files for code fences that end in js. Maybe that's more trouble than it's worth, though. Just another thought.

mtthgn commented 8 years ago

@michaelrkn I've got a PR opened here to add the ability to lint JS strings as opposed to only files. In the meantime, I'll reference my fork of the gem and get started on linting our snippets.

What's the process normally like for opening PRs? Does the team like "In Progress" PRs to be opened against issues, or would you rather wait to have a completely reviewable PR first?

The Contributing Document mostly talks about opening issues and PRs against the guides content itself, not necessarily the source code.

michaelrkn commented 8 years ago

@mtthgn Awesome! Generally early PRs are better so that you can get feedback on any potential problems before heading too far down the wrong path :)