Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release proposal: standard v7 #404

Closed
feross opened this issue Feb 6, 2016 · 32 comments

Comments

@feross
Copy link
Member

commented Feb 6, 2016

Placeholder to track things for the release of standard v7, in the near future.

Changes

  • Remove "rules" configuration option (#367) from package.json (Reasoning is here)

New rules

Estimated % of affected standard users, based on test suite

Removed rules

@feross feross changed the title Release proposal: standard 7 Release proposal: standard v7 Feb 6, 2016

@feross feross changed the title Release proposal: standard v7 Release planning: standard v7 Feb 6, 2016

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2016

Hmh, interesting - should we remove plugins entirely or keep them around? With the removal of the React rules (in v6) + no way of plugins people using React would have to switch to standard-react or something similar, right?

The purist in me doesn't mind at all: it won't affect me in the slightest. On the other hand: there's a good amount of people using React. Hmm

@feross

This comment has been minimized.

Copy link
Member Author

commented Feb 7, 2016

React style rules were removed, but JSX parsing and even some JSX formatting rules remain in standard v6. :)

@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2016

Cool

@feross feross referenced this issue Feb 7, 2016
25 of 25 tasks complete
@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2016

ping @maxogden @mafintosh @othiym23 @dcposch @Flet @dcousens @jprichardson @xjamundx @watson @rstacruz @reggi @yoshuawuyts @bcomnes @jb55 - we probably want to make sure everyone is on board with this release hah

@kaicataldo

This comment has been minimized.

Copy link

commented Feb 8, 2016

Hey there! Just wanted to let you know I'm working on the no-whitespace-before-property bug and have a PR open for it.

@feross

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2016

@kaicataldo That's super helpful! Thanks!!

@maxogden

This comment has been minimized.

Copy link
Contributor

commented Feb 8, 2016

I agree with everything in #399 (comment) for the record.

Regarding plugins, I've never used them but do we expect people to use custom plugins to override standard behavior (e.g. like this #399 (comment))? Or is it just for extending standard? I'm cool with extending, but not overriding. If it's impossible to enforce one and not the other, then I'd probably remove it (e.g. err on the side of less features that can confuse users in the long term)

@feross

This comment has been minimized.

Copy link
Member Author

commented Feb 8, 2016

The plugins option can't be used to actually set any rules as of eslint v2.0.0. See this note in the 2.0.0 changelog. If we don't expose rules, then plugins can only transform the AST before the linting happens.

@dcousens

This comment has been minimized.

Copy link
Member

commented Feb 8, 2016

ACK on all of the above. No configuration is why I use standard.

@dcousens

This comment has been minimized.

Copy link
Member

commented Feb 8, 2016

IMHO @feross if 6.0.0 was the only 'configurable' version of standard, then I'd be happy with leaving it as is, bumping to 7.0.0 with all of standard-strict enabled.

@LinusU

This comment has been minimized.

Copy link
Member

commented Feb 16, 2016

@feross Why remove no-regex-spaces? I couldn't find any reasoning or an issue on it...

@timoxley

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2016

we probably want to make sure everyone is on board with this release hah

For v7 & future major releases I'd like to suggest a policy of at least a few weeks of release candidate time to collect feedback based on running the latest versions against real codebases.

@jprichardson

This comment has been minimized.

Copy link
Member

commented Feb 17, 2016

For v7 & future major releases I'd like to suggest a policy of at least a few weeks of release candidate time to collect feedback based on running the latest versions against real codebases.

👍 Agreed. I think this is important because part of the job of standard is to help us have practical, common-sense (forgive the politico speech, haha) defaults when writing code. This would allow most of us to test it out and be sure that these rules make sense. And if there's disagreement, Lord @feross could then make the final call 😛

@maxogden

This comment has been minimized.

Copy link
Contributor

commented Feb 17, 2016

all hail benevolent lord feross! ⚔🛡🏆🍻👑

On Wednesday, February 17, 2016, JP Richardson notifications@github.com
wrote:

For v7 & future major releases I'd like to suggest a policy of at least a
few weeks of release candidate time to collect feedback based on running
the latest versions against real codebases.

[image: 👍] Agreed. I think this is important because part of the job
of standard is to help us have practical, common-sense (forgive the
politico speech, haha) defaults when writing code. This would allow most of
us to test it out and be sure that these rules make sense. And if there's
disagreement, Lord @feross https://github.com/feross could then make
the final call :)


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

Sent from my phone

@dcousens

This comment has been minimized.

Copy link
Member

commented Feb 18, 2016

Was a milestone set for the release date? It seemed like the release culminated a lot of merging over the course of 2-3 days and then it was released without further feedback? :)

Not fussed, all hail, but, might have saved us the fat arrow parentheses screw up.

@feross

This comment has been minimized.

Copy link
Member Author

commented Feb 20, 2016

The release candidate suggestion is a good one. I've done that in the past:

$ npm show standard versions

[ '1.0.0',
  '1.0.1',
  ...
  '3.0.0-alpha',
  '3.0.0-beta',
  '3.0.0-beta2',
  '3.0.0-beta3',
  '3.0.0-beta4',
  ...
  '5.0.0-0',
  '5.0.0-2',
  '5.0.0-3',
  '5.0.0-4',
  '5.0.0-5',
  '5.0.0-6',
  '5.0.0-7',
  '5.0.0-8',
   ...

Will do that for v7.

@feross feross removed the blocked label Mar 16, 2016

@sotojuan

This comment has been minimized.

Copy link

commented Apr 4, 2016

I'd love a fix for #416!

@simonratner

This comment has been minimized.

Copy link

commented Apr 11, 2016

Comment regarding promise/catch-or-return -- does it allow for yield as well as return?
Yielding promises is common when using something like tj/co.

@xjamundx

This comment has been minimized.

Copy link

commented Apr 11, 2016

@simonratner It doesn't yet, but might be easy to add. Can you add an issue over there:
https://github.com/xjamundx/eslint-plugin-promise/issues

@simonratner

This comment has been minimized.

Copy link

commented Apr 11, 2016

Accept yield in catch-or-return: xjamundx/eslint-plugin-promise#9

@feross

This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2016

Going to punt on catch-or-return for v7.0.0 because it is causing a lot of packages to fail, even with allowThen enabled. We can discuss it for later.

@feross

This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2016

Hey everyone!

standard version 7.0.0-beta.0 was just released with the following changes. Please install it and give it a try! If everything looks good, this will become standard v7!

npm install -g standard@7.0.0-beta.0

Changes

  • Remove "rules" configuration option (#367) from package.json (Reasoning is here)

New rules

Estimated % of affected standard users, based on test suite

Removed rules

@sotojuan

This comment has been minimized.

Copy link

commented Apr 24, 2016

With this AVA test:

test('async', async t => {
  let x = await fn()

  t.is(x, 'hi')
})

I get: Parsing error: Unexpected token t. I'm guessing async/await is not supported yet?

@feross

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2016

@sotojuan This is expected. It didn't work with the previous version of standard either. See https://github.com/feross/standard#can-i-use-a-custom-js-parser-for-bleeding-edge-es6-or-es7-support

@feross

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2016

Happy to see there are no complaints about v7 yet. Let's release the current version as v7 in 48 hours from now. So April 30.

@yormi

This comment has been minimized.

Copy link

commented Apr 28, 2016

When forgetting a space after the function keyword it says:

32:54  error  Missing space after *

I think it was clearer before !?

Moreover, npm is telling me the package is invalid. I'm not sur if that can be usefull info for you guys:

└── standard@7.0.0-beta.0  invalid
@sotojuan

This comment has been minimized.

Copy link

commented Apr 28, 2016

@feross Got it, thanks!

@feross

This comment has been minimized.

Copy link
Member Author

commented Apr 29, 2016

@yormi It only shows "Missing space after *." when you're using a function *, right? With a normal function, the error message remains the same "Missing space before function parentheses."

The "invalid" message you're getting just means that you have a different version of standard in your package.json than the one that is currently installed. It's not an issue since you just installed v7 to test it out. You can make it go away if you update your package.json with:

npm install standard@beta --save-dev
@yormi

This comment has been minimized.

Copy link

commented Apr 29, 2016

It was with an async function (). I guess it's kind of the same semantic but since the written code is different, it might be nice to have a specific message, or if it's complicated to achieve, a mention somewhere in the README.

@feross

This comment has been minimized.

Copy link
Member Author

commented May 2, 2016

@yormi This sounds to me like it's caused by babel-eslint which I assume you're using.

@feross

This comment has been minimized.

Copy link
Member Author

commented May 2, 2016

Alrighty, I'm going to upgrade eslint to 2.9.0 and add two new uncontroversial rules that were added in 2.9.0.

http://eslint.org/docs/rules/no-unsafe-finally
http://eslint.org/docs/rules/no-useless-computed-key

Then I will release standard v7.

@feross

This comment has been minimized.

@feross feross closed this May 2, 2016

@feross feross changed the title Release planning: standard v7 Release proposal: standard v7 Jul 12, 2016

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.