SassDoc / sassdoc

Release the docs!
http://sassdoc.com
MIT License
1.41k stars 56 forks source link

Linted the whole codebase to standard #482

Closed KittyGiraudel closed 7 years ago

KittyGiraudel commented 7 years ago

I think there is a test not passing and I’m not sure why.

KittyGiraudel commented 7 years ago

Damn. I recall we agreed we would move to standard. 😁

The reason why I would like to use it is that it would make sure:

Do you feel like you can cope with the lack of semicolon or shall we use semistandard? I know @valeriangalliat was very much pro standard. So am I. Not sure for @FWeinb.

pascalduez commented 7 years ago

That was the issue: #447. Where we agreed on Standard, but not clearly on semi or not.

I'm a bit worried about such massive changes, and as we can see the automation broke a few things along the way. Given we all have barely much time to allow to the project, if we can limit the possible sources of issues, might be good.

I kind of like the way I setup here: https://github.com/SassDoc/broccoli-sassdoc/blob/master/.eslintrc so you have all the benefits of Standard, but with the confort of ESLint.

Anyway, I will follow the consensus, no big deal.

KittyGiraudel commented 7 years ago

I'm a bit worried about such massive changes, and as we can see the automation broke a few things along the way. Given we all have barely much time to allow to the project, if we can limit the possible sources of issues, might be good.

It wasn’t automation. I removed all semi-colons from the project with a search and replace. :D

KittyGiraudel commented 7 years ago

I’m not too worried as we have good test coverage. Also there will be no direct release following this change.

pascalduez commented 7 years ago

I would like to make a release with the dependencies upgrades, so better not merge this one right now ;)

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 94.618% when pulling 9152b36bef843fd430b652d6643b15b21fb237be on feature/standard into 8aad8c1e9f5d883c51f19786483e949bb816fe8f on master.

pascalduez commented 7 years ago

Once again, unit tests ❤️

KittyGiraudel commented 7 years ago

I would like to make a release with the dependencies upgrades, so better not merge this one right now ;)

Makes sense. :)

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.005%) to 94.618% when pulling 1fecc421592ba4605743c5c902a91c3ce2512202 on feature/standard into 8aad8c1e9f5d883c51f19786483e949bb816fe8f on master.

pascalduez commented 7 years ago

One pain point about using standard that way, we need yet another editors plugin to have code linting in place, since ESLint is all hidden underneath.

KittyGiraudel commented 7 years ago

I’m not sure what you mean.

pascalduez commented 7 years ago

I use plugins in Atom or Vim to get "in place linting" (in the code directly), not just by running the npm command. Those ESLint plugins works independently of the config/preset you have. Standard included.

But since standard is a package on top of ESLint somehow abstracting it, and we have no .eslintrc or eslint key in package.json anymore those plugins don't work. And we need yet another plugin to make it work.

Using Standard the ESLint way is just (if I'm not mistaken):

// .eslintrc
---
extends: "standard"
KittyGiraudel commented 7 years ago

We could add back the .eslintrc file?

pascalduez commented 7 years ago

Anyway, plugins installed, all good. Leave it as is.

pascalduez commented 7 years ago

sassdoc@2.2.0 and sassdoc-theme-default@2.5.2 released. Should get rid of most of the security or deprecation warnings.

Good to go for this MR.

pascalduez commented 7 years ago

So I rebased and fixed the whole damn thing :) Let's see what the CI says.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.02%) to 94.345% when pulling e124d1894f9a9fd92c0fc1ee61d4cc1950dad82e on feature/standard into 2ea9ede361ebcdea2c68538bcb5afc2f2085d267 on master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.02%) to 94.345% when pulling a7779565adc8d63013d4ec338b94bc72691ba934 on feature/standard into 2ea9ede361ebcdea2c68538bcb5afc2f2085d267 on master.