cerebral / cerebral-debugger

Debugger for Cerebral
http://cerebraljs.com/docs/introduction/debugger.html
MIT License
33 stars 16 forks source link

Introduces prettier and changes from standard cli to eslint #52

Closed mikaelbr closed 6 years ago

mikaelbr commented 6 years ago

This PR introduces prettier with same settings as cerebral main repo. To achieve this in a consistent way it changes from using the standard convenience cli to use eslint directly.

Also adds eslint fixing on git hooks and prettier on non-javascript-files.

mikaelbr commented 6 years ago

Did some updating, and IMO improvements in the eslint configuration. Extracted the prettier settings from eslint config to be placed in prettierrc so it can be used in editor settings and such. Also added a .vscode editor setting files with formatOnSave true for enforcing styles actively while coding.

Maybe we should also consider to add css and md linting to Cerebral.

Note, this isn't introducing linting to CSS and md files, just pretty printing (parsing + re-print). In some way it validates the code, as it parses ASTs, and often this is enough. But it isn't as strict as linting. For that you could do something like stylelint for CSS.

Guria commented 6 years ago

I don't like editor specific files in shared repo. Don't all use same editors. If we adding vscode config, does it mean we should place webstorm, atom, vim, emacs configs as well? We still can have a section in readme with recommended settings.

mikaelbr commented 6 years ago

Sure. There are trade-offs, different opinions on the subject. Dropped commit.

mikaelbr commented 6 years ago

Editors should be able to use .eslintrc.json directly. Just use the eslint plugin for your editor and at least for javascript it works out of the box.

As you say this is oriented around linting. Linting tools for editors can read eslint configuration inherited by prettier config, but again pretty printing isn't the same as eslint.

If you use it through eslint configuration the configurations wouldn't be read for formatting things as CSS files and markdown files. For this project that would mean that 16% of the code base would either not be pretty printed, or pretty printed with different settings.

I would argue that this is a more idiomatic way of using prettier, and that the Cerebral project should change it's setup if the point is to have it the same. Ideally, I'd say that using linting for style rules when having AST pretty printing is unnecessary and a overuse of linting.

henri-hulski commented 6 years ago

Yeah you're right. It seems the recommendations changed since I set up Cerebral. I think the recommended option is new.

mikaelbr commented 6 years ago

What one could do is to extract the eslint config to a seperate repo/module and reuse (extend through eslint) on Cerebral and other projects. Easier to maintain and be consistant. Though cerebral I guess is a mono-repo, there are a lot of other repos as well. AFAIK, you'd have to duplicate the prettier rc-file, though.

christianalfoni commented 6 years ago

@mikaelbr You have time for a rebase here? :) Will release new version of debugger ASAP

mikaelbr commented 6 years ago

I’ll try to find the time within the end of the day. If it’s just squashing you are thinking of, you can also do that through the GH ui.

henri-hulski commented 6 years ago

@mikaelbr BTW thanks for inspiration. cerebral/cerebral#1295

mikaelbr commented 6 years ago

@christianalfoni & @henri-hulski : How do you want this in regards to commits? Should I squash something, reorder, reword?

mikaelbr commented 6 years ago

Btw, @christianalfoni , I think I got all merge conflicts, but would love it if you did a quick check and see that your latest changes is still good.

henri-hulski commented 6 years ago

For me the commits are OK. Other than in Cerebral here we don't use the commit messages to create the release notes.

christianalfoni commented 6 years ago

Great, I will do a small check, thanks a bunch @mikaelbr :-)