Skip to content

Commit

Permalink
Expecting me to alphabetize my variable declarations is both ridiculo…
Browse files Browse the repository at this point in the history
…us and unnecessarily tedious. I'm never going to do that and I don't expect anyone else to.
  • Loading branch information
sizzlemctwizzle committed Nov 27, 2013
1 parent 30c15c2 commit 715f11d
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion STYLEGUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Variable declarations without value should be initialized to `null`

All variable statements should be the first statements within the function body.

Each variable declaration should be separate, on its own line, in alphabetical order, e.g.,
Each variable declaration should be separate, on its own line, e.g.,
```javascript
var baz = 'meep';
var foo = true;
Expand Down

14 comments on commit 715f11d

@sizzlemctwizzle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until you can get the team to agree to this, the person writing most of the code trumps any other individual in these matters.

@cletusc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem with the change. Alphabetizing blindly can lead to issues anyhow. Personally, I only alphabetize objects properties and only when they need to break onto their own line. I take the same approach with CSS properties and HTML attributes. I have a shortcut on Sublime--select the lines needing alphabetized, then use a "sort lines" command I added to the context menu.

@sizzlemctwizzle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo alphabetizing is something you do with your CD and DVD collections, not with code. Finding things is what search functions are for. Alphabetizing is an unnecessary barrier to entry. If I was a random dude thinking about contributing and saw that requirement, I'd definitely jog on.

@cletusc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be a requirement, I agree. As I mentioned above, I only do that for object properties, CSS properties, or HTML attributes, and that is just a personal preference that rubbed off on me from Firebug's automatic alphabetizing of the three I mentioned.

@joesimmons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have asked the others about this before changing it yourself.

That being said, I'm fine with it being removed. I, personally, will still try to alphabetize them when I can. I find it much easier to find variables and change them when my mind can just zero in on them automatically instead of my eyes having to search every single variable until I find it.

@sizzlemctwizzle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have asked the others about this before changing it yourself.

You should have asked the others about this before putting it there in the first place.

@joesimmons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should have asked the others about this before putting it there in the first place.

Uh, hello, we agreed to use Crockford's conventions, and that was one of them.

@sizzlemctwizzle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, never noticed that part.

@joesimmons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not trying to be mean or anything, but we did agree to this, so removing it after we agreed upon it is quite insulting.

And again, I don't mind it being removed. It's all good. But let's chat beforehand next time.

@sizzlemctwizzle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we did agree to this

Afaik only I agreed to use Crockford's conventions and I hadn't noticed that ridiculous alphabetizing bit until now. Silence doesn't equal consensus.

But normally I wouldn't go ahead and remove something just because I didn't like it. This rule was just as ridiculous as requiring me to write code while standing on my head, so I thought it was embarrassing to have in our guide and grounds for immediate removal.

@cletusc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not trying to be mean or anything, but we did agree to this, so removing it after we agreed upon it is quite insulting.

I created the style guide in the issue-19 branch in order to get feedback from everyone, and there have already been changes to it as discussed on the mailing list. You added your own version and pushed to master immediately without any input from anyone else, which wasn't cool and equally as insulting.

Other than sizzle (sans the agreement on this issue), no one else agreed on the new version, I just haven't had the chance to fully look at this version.

@sizzlemctwizzle
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and equally as insulting.

Just nitpicking, but I wouldn't equate changing one line to remove something most people would think was ridiculous with rewriting the whole thing. Maybe I should have consulted others before making the change. Sometimes I'm arrogant bastard. My bad.

I haven't had the chance to review both guides in full, since I've been busy writing code, but one thing I'd like to see in the issue-19 guide, that was present in Joe's version, is that variable declarations should be put at the top of a function scope and any variable set later on initialized to null. This makes debugging things easier since a null value will tell you that the variable exists but wan't set, and an undefined lets you know that the variable doesn't exist and maybe you fat-fingered it like I do.

@joesimmons
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added your own version and pushed to master immediately without any input from anyone else, which wasn't cool and equally as insulting.

Sorry about that. I copied basically everything over, except the bit about the truthy/falsy values. I don't agree with that, maybe we should have a discussion.

@simonzack
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really care about alphabetizing much, I usually group related variables together.

Please sign in to comment.