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

Publish new release with support for updated style guide #24

Open
2 of 3 tasks
addyosmani opened this issue Sep 26, 2016 · 10 comments
Open
2 of 3 tasks

Publish new release with support for updated style guide #24

addyosmani opened this issue Sep 26, 2016 · 10 comments

Comments

@addyosmani
Copy link
Contributor

addyosmani commented Sep 26, 2016

  • Add support for new style guide to master
  • Test master with Google OSS projects (e.g sw-toolbox) that use the style guide to determine state of breakage, if any. What else might be useful to capture in the changelog notes
  • Tag new major release and publish to npm

cc @philipwalton @gauntface as an fyi

@philipwalton
Copy link
Contributor

I updated a few projects I work on with the new style guide and found three major areas of breakage:

  1. comma-dangle: New styles require trailing commas in arrays and object definition blocks. Many of our OSS projects do not do this.

  2. arrow-parens: The styleguide recommends using parens around single-argument arrow functions. Several of the projects I've seen extending this config include rules that actually require no parens in those cases. This may not cause breakage (if the rule is an override), but we should be consistent.

  3. no-invalid-this: I don't think this is a change for the previous Google style guide, closure compiler has always been pretty strict about where you could use this for optimization reasons.

@gauntface
Copy link

Is say make it a major release and don't worry about the breakage. If it's best practice / style then go with it.

@BrockWills
Copy link

Is there any way to view the new styleguide? The style guide linked in the readme is no longer accurate with these changes

@addyosmani
Copy link
Contributor Author

It hasn't been published outside of Google just yet. We're working on it but it's going to take a bit :)

@appsforartists
Copy link
Member

CC @dongbohu

@gauntface
Copy link

Worth updating the readme to hint at using the eslint:recommended config as well as Google?

Found a few things that were nice to have from eslint:recommnded - forced env + ecmascript config.

@addyosmani
Copy link
Contributor Author

I think so. Could you file an issue and we can get that tackled?
On Tue, Oct 18, 2016 at 7:16 PM Matt Gaunt notifications@github.com wrote:

Worth updating the readme to hint at using the eslint:recommended config
as well as Google?

Found a few things that were nice to have from eslint:recommnded - forced
env + ecmascript config.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#24 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGxaUijiOL73qPTpYT8wrwqSSRMLBXmks5q1X1lgaJpZM4KHHen
.

@philipwalton
Copy link
Contributor

Created #27. I'll send a PR for this shortly.

@ilyaigpetrov
Copy link

ilyaigpetrov commented Nov 29, 2016

0.7.1 was published on NPM.
Please, update url of "Google JavaScript style guide" in README.md
@addyosmani and all the team, thank you.

@goatandsheep
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants