Skip to content

Conversation

jzaefferer
Copy link
Member

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
Copy link
Member

dmethvin commented Feb 5, 2015

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
Copy link
Member

gnarf commented Feb 5, 2015

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 [email protected]
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
#105 (comment)
.

@scottgonzalez
Copy link
Member

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 jquery/content#8.

@scottgonzalez
Copy link
Member

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

@arthurvr
Copy link
Member

Sure!

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.

Also removes two unnecessary blank lines in unrelated code example.

Closes jquery#105
@jzaefferer jzaefferer merged commit a1fbe20 into jquery:master Mar 11, 2015
@jzaefferer jzaefferer deleted the remove-onevar branch March 11, 2015 12:50
@jzaefferer
Copy link
Member Author

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

@timmywil
Copy link
Member

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.

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

Successfully merging this pull request may close these issues.

7 participants