Release proposal: standard v6 #399

Closed
feross opened this Issue Feb 4, 2016 · 17 comments

Projects

None yet

9 participants

@feross
Owner
feross commented Feb 4, 2016

The goal of this release is to make standard faster to install, and simpler to use.

  • Remove standard-format (#340) (#397)
    • Eliminates 250 packages, and cuts install time in half!
    • For npm2, install went from 20 secs to 10 secs.
    • For npm3, install went from 24 secs to 12 secs.
  • Remove React rules (#351) (#367) (feross/eslint-config-standard-react#13)
    • JSX is still supported, but standard no longer assumes that JSX === React, so users can use alternatives like virtual-dom or deku.
    • Rules come from eslint-config-standard-jsx instead of eslint-config-standard-react now.
  • Expose eslint configuration via command line options and package.json "standard" property
  • Upgrade to ESLint v2 (stable release is imminent)
    • There may be slight behavior changes to existing rules. When possible, we've noted these in the "Rule changes" section.
  • Rule changes [% of standard users with new rule failures is noted in brackets]
    • New rule: Disallow __dirname/__filename string concatenation (#403) (no-path-concat) [5%]
    • New rule: Require parens in arrow function arguments
 (#309) (arrow-parens) [5%]
    • New rule: Ensure that new Promise() is instantiated with the parameter names
      resolve, reject (#282) (promise/param-names) [1%]
    • New rule: Enforce Usage of Spacing in Template Strings (template-curly-spacing) [1%]
    • New rule: Template strings are only allowed when necessary, i.e. template string features are being used (eslint got stricter: eslint/eslint#5147) [1%]
    • New rule: Better dead code detection after conditional statements (eslint got stricter) [1%]
    • New rule: Enforce spaces around * in yield * something (#335) (yield-star-spacing) [0%]
    • New rule: Disallow labels on loops/switch statements too (made rule stricter) (no-labels) [0%]
    • New rule: Disallow unnecessary constructor (no-useless-constructor) [0%]
    • New rule: Disallow empty destructuring patterns (no-empty-pattern) [0%]
    • New rule: Disallow Symbol Constructor (no-new-symbol) [0%]
    • New rule: Disallow Self Assignment (no-self-assign) [0%]
    • Relax rule: parseInt() radix rule because ES5 fixes this issue (#384)
 (radix) [0%]
  • Fix ESLint v2 bugs (possible unintentional breakage):
    • "no-unexpected-multiline" false positive (eslint/eslint#5148) Fixed with PR 👍
    • no-return-assign behavior changed with arrow functions (eslint/eslint#5150) - RELEASED ANYWAY, NOTED UNDER "KNOWN ISSUES"
  • Improve test suite to test rule changes against every package on npm
    • For sanity, limiting to packages with at least 4 dependents. Around ~400 packages.
@feross feross added the v6 label Feb 4, 2016
@rstacruz
Collaborator
rstacruz commented Feb 4, 2016

\o/

@yoshuawuyts
Collaborator

@feross
Owner
feross commented Feb 4, 2016

Merged the v6 branch into master, so merging PRs for v6 is easier.

@feross
Owner
feross commented Feb 5, 2016

Going to try to get a beta release out on npm tomorrow.

@feross
Owner
feross commented Feb 6, 2016
@feross feross closed this Feb 6, 2016
@jprichardson
Collaborator

Standard is by far one of my favorite projects and I think it has really helped to improve the code quality in the JavaScript ecosystem. @feross I thank you for all of your hard work put into it.

With that being said, I'm concerned that enabling standard to be configurable https://github.com/feross/standard/blob/master/CHANGELOG.md#expose-eslint-configuration-via-command-line-options-and-packagejson was a mistake. See https://github.com/feross/standard#can-you-make-rule-x-configurable:

No. The point of standard is to save you time by picking reasonable rules so you can spend your time solving actual problems. If you really do want to configure hundreds of eslint rules individually, you can always use eslint directly.

If you just want to tweak a couple rules, consider using this shareable config and layering your changes on top.

Pro tip: Just use standard and move on. There are actual real problems that you could spend your time solving! :P

I understand that we as developers need to evolve projects, take risks, listen to feedback, and iterate. However, this change feels antithetical to what standard stood for and what made it great. It seems entirely possible that I'm misunderstanding these changes or the importance of these changes and what they entail. My fear now is that I'll open up a package.json and see standard with a bunch of different config options which brings it all back to everyone's custom eslint configuration. Before, I could open up a project's package.json and if I saw standard, I knew what to expect and that's part of what made standard great.

@feross I respect and understand that standard is your project. I just wish maybe you would have sought a bit more feedback on that change (perhaps I missed it?).

I will definitely upgrade to Standard v6, continue to use and promote Standard, I just wanted to share this perspective.

Thanks again for your hard work on this!

@yoshuawuyts
Collaborator

Crap, I fully missed this - I was moving cities the last couple of days and somewhere this slipped. I agree with @jprichardson, I feel like this can be, and thus will be abused fully

{
  "standard": {
    "rules": {
      "semi": [2, "always"]
    }
  }
}

If someone does the above, then saying that a project uses standard becomes meaningless - I feel this is different from having react / jsx / mocha support as external packages. If configuring those is painful, they should be resolved in their respective packages. Perhaps config should only be accepted for the rules they touch - the default standard shouldn't be messed with I reckon, if you want to do that it should be done from a (new) plugin.

I kind of like being grumpy old farts that curate the one way to do things and if you don't like it shoo and configure your own. This doesn't feel right to me.

@maxogden
Collaborator
maxogden commented Feb 7, 2016

agree with last two comments. leave it up to other modules to add
configuration

On Saturday, February 6, 2016, Yoshua Wuyts notifications@github.com
wrote:

Crap, I fully missed this - I was moving cities the last couple of days
and somewhere this slipped. I agree with @jprichardson
https://github.com/jprichardson, I feel like this can be, and thus will
be abused fully

{
"standard": {
"rules": {
"semi": [2, "always"]
}
}
}

If someone does the above, then saying that a project uses standard
becomes meaningless - I feel this is different from having react / jsx /
mocha support as external packages. If configuring those is painful, they
should be resolved in their respective packages. Perhaps config should only
be accepted for the rules they touch - the default standard shouldn't be
messed with I reckon, if you want to do that it should be done from a new
package.

I kind of like being grumpy old farts that curate the one way to do
things and if you don't like it configure your own
. This doesn't feel
right to me.


Reply to this email directly or view it on GitHub
#399 (comment).

Sent from my phone

@feross
Owner
feross commented Feb 7, 2016

Thanks for the feedback guys. Better late than never, I suppose.

This whole thing got me thinking about why people like standard in the first place. I think there are two main reasons people like it:

  1. Eliminates need to add .eslintrc files and keep them in sync across all projects
  2. Tells you what to think, eliminates bikeshedding

I originally made standard solely for reason 1, to enforce style on the various WebTorrent repos. But I think most people use it for reason 2.

Adding a way for the super-obsessed to set an eslint plugin for whatever flavor-of-the-week JS thing they're using doesn't really affect my use case (reason 1). But it definitely affects reason 2, and makes "this project uses standard" mean way less. And that's not cool.

I think ya'll are right. I done flubbed it up.

There were issues about this in #386, #367, and #371, for plugins, rules, and env configs, respectively. Wish we could have bikeshed this before I released v6! Oh well.

So, just to recap.

The reason for adding rules was so folks like @rstacruz and @HenrikJoreteg could have an easier time using React alternatives like deku and virtual-dom. But, now that the React rules are removed in v6, there is no need to globally disable specific rules. Let's remove this in v7.

The reason for adding plugins was so folks wouldn't need to create/use a standard fork just so they can pass a simple option like { plugins: ["flowtype"] } into eslint. And we already allow passing through the { parser: "babel-eslint" } option to eslint, and this seemed along the same lines as that.

The reason for adding env was so folks writing code for an environment like mongo or mocha, where there are assumed global variables could turn this on. It doesn't change any rules – just makes some errors about using an undefined variable go away. But honestly, making mocha users explicitly add /* eslint-env mocha */ is fine, and it's good documentation anyway since they're not writing require('mocha').

So, I'm 👍 to doing a quick v7 release to remove rules and env. Any thoughts on plugins?

@feross feross referenced this issue Feb 7, 2016
Closed

Release proposal: standard v7 #404

10 of 10 tasks complete
@feross
Owner
feross commented Feb 7, 2016

Could folks please chime in with feedback on the standard v7 release proposal: #404

@dcousens
Collaborator
dcousens commented Feb 8, 2016

IMHO a release that removes rules would be a patch. Not a new 7.0.0.
This is the sole reason I use standard, no configuration. I know what to expect.
You done goofed it up @feross!

But otherwise amazing work on pumping out 6.0.0.

@feross
Owner
feross commented Feb 8, 2016

😜

@jescalan
jescalan commented Feb 8, 2016

+1 for keeping plugins! 😁

@rstacruz
Collaborator
rstacruz commented Feb 8, 2016

sounds cool.

if anyone disagrees, they're always free to drop down to eslint + eslint-config-standard.

@dcousens
Collaborator
dcousens commented Feb 8, 2016

if anyone disagrees, they're always free to drop down to eslint + eslint-config-standard.

At which point, the intention is clear as day and they're not using standard, but their own concoction anyway.

@craigmichaelmartin craigmichaelmartin referenced this issue in brave/browser-laptop Feb 12, 2016
Closed

Use own .eslintrc instead of standard #273

@jmalonzo

I just want to chime in this quickly as I've recently switched our project to Standard. I believe Standard is great if you are starting a project from scratch. But moving an existing codebase to it, it gets daunting as you will instantly get thousands of errors from the get go. Nobody wants to spend their days/weeks fixing lint errors (and nobody wants to fix these errors blindly either).

This is where rules/globals/env really help as it helped me move from one class of errors to another one and making small wins in the process. Also, some of Standard's defaults are debatable within a team setting (e.g. semi and indentation level) and something that probably would slow a team down if introduced straightaway without some discussion. Again this is where 'rules' help as I can introduce these features slowly over time to the team without any friction or useless debates about style.

I hope that sheds some light on some of the use-cases of rules, at least. globals - not everyone uses a module bundler and not all projects are in the npm registry, so globals help there.

Cheers.

@HenrikJoreteg

I'm with @rstacruz on this keep standard strict. You can always customize with eslint + eslint-config-standard if you want a non-standard, standard (see how silly that sounds 😜).

Anyway, all the good feels: https://twitter.com/HenrikJoreteg/status/697294740480352256

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment