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

Adopt standard module (for new projects) #138

Closed
jzaefferer opened this issue Apr 25, 2016 · 14 comments
Closed

Adopt standard module (for new projects) #138

jzaefferer opened this issue Apr 25, 2016 · 14 comments

Comments

@jzaefferer
Copy link
Member

Here's a suggestion to put an end to all style bikeshedding, by adopting https://www.npmjs.com/package/standard - at least for new projects.

I've been using that module as the only linter on a decent size projects for a year now and recently started adopting smaller repos on GitHub that I own or maintain to use it as well. I don't miss semicolons at all (you can read about how there are no technical reasons for using them, please don't discuss those here) and everything else is very straightforward.

After ESLint joining the jQuery Foundation, officially endorsing this module would send a good signal to the wider JavaScript community. Let's consolidate!


PS: Longer version of that last one: Let's consolidate comprehensive and correct solutions!

@scottgonzalez
Copy link
Member

I really don't like the lack of semicolons, but I really do like the inability for us to every bikeshed about style again.

@dmethvin
Copy link
Member

You mean Foundation-wide?

@gibson042
Copy link
Member

The "never start a line with…" exceptions are insufficient to cover all ASI edge cases (including one that's in the spec), and further, it is incomplete with respect to the contents of our style guide, so we'd still be adding (and bikeshedding) additional restrictions (most notably spaces inside statement and/or call parentheses, but also line wrapping, mandatory statement braces, object literals, and a handful of others).

In short, standard feels unfinished. I don't think we should consider adopting it until it is much closer to defining a single unambiguous representation for any code by explicit rules rather than those inferred from examples.

@scottgonzalez
Copy link
Member

The "never start a line with…" exceptions are insufficient to cover all ASI edge cases (including one that's in the spec)

The semicolons are my biggest concern, though that could be addressed by using semi-standard.

it is incomplete with respect to the contents of our style guide, so we'd still be adding (and bikeshedding) additional restrictions (most notably spaces inside statement and/or call parentheses, but also line wrapping, mandatory statement braces, object literals, and a handful of others).

The idea is that we wouldn't do any of those things; our style guide becomes "Follow JavaScript Standard Style."

@scottgonzalez
Copy link
Member

You mean Foundation-wide?

This has come up before because of the scope expansion of the foundation. Having any style guide on contribute.jquery.org become incorrect a very long time ago but nobody seemed to want to change anything. It seems clear that we'll never have a foundation-wide style guide.

I'm interested in what you think the scope of the current style guide is.

@gibson042
Copy link
Member

The semicolons are my biggest concern, though that could be addressed by using semi-standard.

"Semi-standard" being https://github.com/Flet/semistandard ?

The idea is that we wouldn't do any of those things; our style guide becomes "Follow JavaScript Standard Style."

If "JavaScript Standard Style" is to be understood as exactly bulleted rules in https://github.com/feross/standard/blob/65efd35a3dca6c633acf9e9df36b67666f4fe447/RULES.md and exclude inferences from examples and the sample file, then that won't work because of how much is unspecified. For instance, both of the following comply:

// compact
if (true) options = {foo:bar(baz)}
// spacey
if ( true ) options = { foo : bar ( baz ) }

I don't think we should adopt standard without such issues being addressed, and even then will almost certainly end up imposing further restrictions (e.g., always/never use braces for single-line conditional and iteration statement bodies), subject to bikeshedding. However, the surface area would be much reduced, and for that I could even give up semicolons.

@scottgonzalez
Copy link
Member

"Semi-standard" being https://github.com/Flet/semistandard ?

yes

If "JavaScript Standard Style" is to be understood as exactly bulleted rules in https://github.com/feross/standard/blob/65efd35a3dca6c633acf9e9df36b67666f4fe447/RULES.md

JavaScript Standard Style is to be understood as the set of rules that standard enforces. If a verbose document of the rules is what you want, I don't think the maintainers would object to a PR to add such a document.

For instance, both of the following comply:

In fact, both will fail the linter. The former comes very close to being valid though.

always/never use braces for single-line conditional and iteration statement bodies

I'm almost always on board with this because of the potential for accidental bugs. However, standard actually only allows the braces to be excluded if the body is on the same line as the condition.

This is valid:

if (true) options = { foo: bar(baz) }

This is invalid:

if (true)
  options = { foo: bar(baz) }

Now, we may want spaces inside compact objects but fully compact objects will pass the lint check.

valid (consistent spacing):

{ foo: bar(baz) }
{foo: bar(baz)}

invalid (inconsistent spacing):

{ foo: bar(baz)}

As much as I would like to force one style, I really like the idea of not having config files and not being able to bikeshed about style within our projects (bikeshedding would have to be delegated to standard).

@jzaefferer
Copy link
Member Author

I'm interested in what you think the scope of the current style guide is.

While that question was to Dave, my take is: It is an option for our projects to adhere to, which currently only a subset of existing projects do. I'm suggesting to instead endorse standard, deferring style discussions to that project.

For example, the details @gibson042 brought up above could be discussed at https://github.com/feross/standard/issues

In general I don't think an "incomplete" style guide is actually a problem. There are so many ways to write JavaScript that its impossible to specify a single style. The important part is to produce consistent code, where discussions about style are reduced to a minimum.

@dmethvin
Copy link
Member

As far as this particular issue, we're not going to tell a new project what style guide to follow, which is implied by "Adopt standard module (for new projects)". I think I would tell projects, "Choose a style guide and enforce it with JSCS or ESLint so you don't have to focus on those things during code review."

This has come up before because of the scope expansion of the foundation. Having any style guide on contribute.jquery.org become incorrect a very long time ago but nobody seemed to want to change anything.

If someone wanted to file a PR to add something to the effect of "each project determines their own style guide and often has build tools to help ensure they're followed" that would be great. This info is from a new project contributor's perspective so we probably just need to make them aware of the general existence of style guides and automated checkers.

The details of the "jquery" style guide here should stay because they're used by people both inside outside our organization, but they aren't binding on projects past or future.

I'm interested in what you think the scope of the current style guide is.

It's the definition of the "jquery" style implemented by JSCS. Projects can use that JSCS preset or choose another one if they want.

@markelog
Copy link
Member

markelog commented Apr 26, 2016

So why standard? Why not xo? Why not airbnb/google? Like airbnb code style is much much more popular by any metric like github stars or npm downloads.

@scottgonzalez

The semicolons are my biggest concern, though that could be addressed by using semi-standard.

I was on that road so many times before, after semicolons it will be spaces, or lets change one another rule about body of arrow functions or something or other.

Let's consolidate!

We did! Remember? It is called jslint :). We deferred it to Crockford now we want to deferred to someone else, "...doomed to repeat it" and staff.

Half-serious here, but believe me when i tell you - one way or another It will never stop, until something radical happens like introduction of gofmt analog as part of the node dist. You know, they say 90% of all golang code is following one code-style. I think that what we should promote, i think that what would be real consolidation.

All and all, if someone is interested in my opinion i agree with @dmethvin -

just need to make them aware of the general existence of style guides and automated checkers

Isn't that is what's already happening?

After this discussion, i really don't know what to do with https://github.com/jquery/eslint-config-jquery

@scottgonzalez
Copy link
Member

So why standard?

Consolidation and lack of config.

Why not xo?

It's configurable; from their own docs: "XO is more pragmatic and has no aspiration of being the style."

Why not airbnb/google? Like airbnb code style is much much more popular by any metric like github stars or npm downloads.

Requires config file.

I was on that road so many times before, after semicolons it will be spaces, or lets change one another rule about body of arrow functions or something or other.

This ignores my first comment: "I really don't like the lack of semicolons, but I really do like the inability for us to every bikeshed about style again." I've honestly given up on style debates. At this point, I'll take anything that requires zero work and has full tooling over ever having another long drawn out discussion about personal preference (because apparently only about 0.0000000001% of developers care about error avoidance).

We did! Remember? It is called jslint :)

Honestly, if JSLint weren't encumbered with a bullshit license and it had more tooling, I'd probably be fine settling on that.

We deferred it to Crockford now we want to deferred to someone else

I'll refrain from my comments about the history behind moving away from JSLint as I don't think it's productive in any way.

one way or another It will never stop, until something radical happens like introduction of gofmt analog as part of the node dist. You know, they say 90% of all golang code is following one code-style. I think that what we should promote, i think that what would be real consolidation.

So you'd rather never even try for this? Moving to, and promoting, standard is exactly how this would be accomplished.

@markelog
Copy link
Member

markelog commented Apr 26, 2016

Consolidation

As i said, airbnb (for example) is much more popular.

Requires config file.

Doc for standard is quite controversial, since it says it doesn't require config, but it actually does, like for globals and what files to ignore, which you define in package.json the same exact thing could be accomplished with any sharable config. And it actually ask you to define inline pragmas like for mocha case.

This ignores my first comment

Sorry, i missed that, like this "I've honestly given up on style debates." is very close to my heart, i really hate those discussions too and in every project i come i tell people - to just pick any code style but don't change specific rules, those discussion are so exhausting.

Honestly, if JSLint weren't encumbered...

I sense there is a history there, so not gonna pursue this.

So you'd rather never even try for this?

You got me there, i would really try this, but on the "higher level" so it would be part of node/npm cli.

@jzaefferer
Copy link
Member Author

You got me there, i would really try this, but on the "higher level" so it would be part of node/npm cli.

That's a overhang you're trying to climb. Why not start with the simpler step of the jQuery Foundation endorsing something?

As far as this particular issue, we're not going to tell a new project what style guide to follow, which is implied by "Adopt standard module (for new projects)". I think I would tell projects, "Choose a style guide and enforce it with JSCS or ESLint so you don't have to focus on those things during code review."

Or you could tell them, "If you don't already follow a style guide, use standard." And tell the rest of the world that we're doing that. Then look at how the popularity takes off.

You can also replace standard with something else that is as configuration-free (globals/ignores don't account, those are usually project specific and can't be shared). As long as we use our influence to make a stand.

@dmethvin
Copy link
Member

I created #139 to clarify that we're not trying to create a Foundation-wide style and this guide applies to whatever projects that want to use it.

dmethvin added a commit that referenced this issue Jul 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants