search

jquery / contribute.jquery.org

Developer documentation common to jQuery projects
https://contribute.jquery.org
Other
25 stars 76 forks source link

JS Style Guide: Drop onevar as a requirement #105

Closed jzaefferer closed 9 years ago

jzaefferer commented 9 years ago

There are various reasons why onevar is bad, like always changing unrelated lines when adding or removing variables. jshint actually remove the onevar option recently. Instead of specifying how variables should be declared without onevar, I've removed the entire Assignments section. We can bring it back in the future when there is consensus, if its needed at all.

dmethvin commented 9 years ago

I'm good with this, it just makes onevar optional so we can continue to use it and still be in compliance with the style guide? That said, I've started to come around to the "one var per variable" because of the diff issue.

gnarf commented 9 years ago

I highly support one var per variable, and keeping with the "all at the top" if we are making changes to this section. I understand there is a lot of code using the old way tho...

On Thu, Feb 5, 2015 at 11:54 AM, Dave Methvin notifications@github.com wrote:

I'm good with this, it just makes onevar optional so we can continue to use it and still be in compliance with the style guide? That said, I've started to come around to the "one var per variable" because of the diff issue.

— Reply to this email directly or view it on GitHub https://github.com/jquery/contribute.jquery.org/pull/105#issuecomment-73081847 .

scottgonzalez commented 9 years ago

I'm good with this, it just makes onevar optional so we can continue to use it and still be in compliance with the style guide?

Yes, but ideally all of the jQuery projects (not all of the jQuery Foundation projects) have the exact same styles. There's an important distinction there and I think consistency in the former is important. See also https://github.com/jquery/content/issues/8.

scottgonzalez commented 9 years ago

@jzaefferer No objections in a month. I'd say this is safe to land :-)

arthurvr commented 9 years ago

Sure!

jzaefferer commented 9 years ago

Merged. We can still add more specific var rules later, project-specific or generic.

timmywil commented 9 years ago

We need a replacement for this and I think it should be for all projects. I'd like to be able to enforce some value for http://jscs.info/rule/requireMultipleVarDecl.html or http://jscs.info/rule/disallowMultipleVarDecl.html and that should probably be included in the jquery preset.