The GitHub Styleguide | Hacker News Reader

The GitHub Styleguide

github.com

133 pointsjamesjyu78 comments

I liked their guideline on when to use bang methods like `array.map!` in ruby:

"The names of potentially 'dangerous' methods (i.e. methods that modify self or the arguments, exit!, etc.) should end with an exclamation mark. Bang methods should only exist if a non-bang method exists."

That last sentence finally made me understand why `string.gsub!` takes a bang but `FileUtils.rm_rf(dir)` does not, even though the latter is far more dangerous (in the generic case). It's not just about the danger level of the method but about the existence of a non-dangerous alternative. Now I've started noticing that some gems do not follow this guideline.

first time i heard that was from greg browns Ruby Best Practices book (was probably the best thing I took away from it). At this point (unfortunately) bang means almost nothing, since the reason to use bang seems to be different for every rubyist

Examples of different uses?

The only example I know of is DataMapper that uses bang methods such as `save!` for the methods that change data bypassing the validation mechanism. But I would argue that this use fits the original definition quite nicely.

There are lots of simpler examples too. Consider Array#push or Array#pop. Both make changes to an array in place, but are not ! suffixed due to the requirement you quoted.

You are right, though. Many Rubyists do not follow this convention and often don't even know about it. It's a key convention of the core implementation team, however.

I just realised this is the same rule as for bang commands in Vim and Pentadactyl.

Does Ruby convention follow Lisp in regards to methods ending in a question mark too? In Clojure and Scheme those methods should return an actual boolean value and not truthy values or nil.

Compare `some` and `every?` in Clojure:

http://clojure.github.com/clojure/clojure.core-api.html#cloj...

And in Scheme `member` doesn't have a question mark for this reason, it returns the cdr of the list starting with the item if it's found.

It's a subtle point, and probably trips up newcomers, but I think it's useful, especially since the types of methods and functions in these languages are not as well broadcast as in statically typed languages.

No semicolons in javascript! This is subversive and should be suppressed.

All coffeescript all the time! I guess you can't expect much else from a ruby group.

Their justification attributes a blog post where the author basically says "don't use tools that will break your code if you omit these syntax features". It encourages people to write code that will break when used by others, dictating the tools they can and can't use. It's the equivalent of a child closing their eyes and sticking their fingers in their ears to magically whisk away the person standing next to them.

It's pretty disappointing to see GitHub, a company that a lot of developers (including myself) look up to and will follow, espousing something like this.

They actually don't provide a justification. That blog makes the argument that omitting them is harmless. Neither GitHub nor the blog make an argument for why you should omit them, however.

Yeah you're right. There's no solid reason to do so, in the face of a solid reason not to.

Do you use semicolons in Ruby, Python, shell, or other semicolon-less language where semicolons are allowed? If not, what's your justification for omitting them?

Why is a justification needed?

The justification is to be idiomatic and match the bulk of existing code.

the only tools that will break your code if you don't use semicolons are so old they shouldn't be used for other reasons. At this point, you should really be using either yui compressor, closure compiler, or uglifyjs for reasons that have nothing to do automatic semi-colon insertion or not.

If you are still using something like packer, that bit of advice will take about 5 minutes of work, and give you significant decreasing of file size.

Semi-colons need to be in 1 place, the beginning of a line that starts with a (, since that is the one place that automatic semicolon insertion will really screw you. If it makes you feel good to write a semi colon at the end of every line I don't think anyone is stopping you, but it isn't the language or tools that require it. You can also feel free to explicitly wrap every statement in parens (because it is necessary in a few cases), or end each line with a // after your semi-colon, all these are freedoms the language gives you, but are as necessary as semi-colons.

I haven't been using unnecessary semi-colons in js for years, and have yet to run into a problem.

Their style guide disagrees with mine! KILL THEM ALL!!!

Did you read the article they linked to? I did. I still disagree with the style rule, but I don't feel like I'd win an argument with them about it.

dailyjs did a nice writeup recently http://dailyjs.com/2012/01/19/semicolons/ and isaac (npm guy) wrote a really good piece on it that I like better then mislavs http://blog.izs.me/post/2353458699/an-open-letter-to-javascr....

I don't even have a problem with people who want to put semi-colons all over the place, I am just tired of having to defend myself on why I choose not to. And it feels kind of odd that I have to, because I am not the one adding redundant characters everywhere

The reason you have to defend yourself is that your style is currently less idiomatic than their style. The caveat of non-idiomatic choices is that you have to defend them either indefinitely or until the idioms change, whichever comes first. What I find odd is when people make a non-mainstream choice and aren't willing to put in the effort to back it up - swimming against the stream is great, but you have to be able to explain to people why you're doing it or be tough enough to ignore them when they ask.

When it comes to arguments about semicolons, there are no winners.

Here's a more accurate tl;dr of their JavaScript style guide:

    $ curl -I https://github.com/styleguide/javascript

    HTTP/1.1 301 Moved Permanently
    Location: https://github.com/styleguide/coffeescript

I think it's neat that people can create languages that compile to JavaScript so they can work in an environment that's more their style, but it really irks me when they try to say those languages ARE JavaScript.

This isn't a debate as to whether or not CoffeeScript is better, easier, cleaner, etc. It's a simple A does not equal B statement, and it's starting to remind me of the way my boss says I'm "good at Java".

A good coding guideline should emphasize the importance of "consistency", so if you tell me that..

"..my advice is to insert them (semicolons) immediately before the opening parenthesis or square bracket in any statement that begins with one of those tokens, or any which begins with one of the arithmetic operator tokens /, +, or - if you should happen to write such a statement..." (http://mislav.uniqpath.com/2010/05/semicolons/)

Then I would say it is really not a good idea.

Scanning through the Ruby styleguide for GitHub-specific changes I found this gem:

"The and and or keywords are banned. It's just not worth it. Always use && and || instead."

sad trombone

Indeed, "and" and "or" are excellent for their intended use, for control flow. Like explained by the style guide the Github one is based on. https://github.com/bbatsov/ruby-style-guide

   # boolean expression
   if some_condition && some_other_condition
     do_something
   end

   # control flow
   document.saved? or document.save!

I'm not sold on tossing "and" and "or" out the window, but wouldn't "document.saved? or document.save!" read better as "document.save! unless document.saved?"

Agreed, it is not my example.

I'd agree with you if not for the many subtle and critical bugs I've seen introduced into our production environment due to and/or precedence problems. The increased expressiveness when used correctly is just not worth it.

Can someone explain the use of // instead of /* */ for CSS comments? According to what I've always known, // isn't even valid.

They are using .scss, so // is valid there.

Prefixing each commented line is more efficient in text editing than wrapping things in blocks. Editor support for mappings to comment out sections of text are more consistent in supporting prefixing each line and removing that than wrapping and unwrapping a selection with the block syntax.

Also, that way you can comment out a larger section and subsequently uncomment a part of that section with a single action instead of uncommenting the entire thing, then re-commenting the part that you still want commented out.

Ah, gotcha. I read "CSS" and just, you know, assumed "CSS".

Although any compressor or minifier will strip out comments, '//' comments in SCSS will be ignored even if you choose not to compress.

It's a really insignificant detail in and of itself, and zefhous' comment provides a far more substantial rationale.

  Use def self.method to define singleton methods.
  ...
  # Also possible and convenient when you
  # have to define many singleton methods.
  class << self

I would argue against using "class << self" especially when you have many singleton methods. If the class is large enough, it's easy to miss the "class << self" and incorrectly read a class method as an instance method.

Usually you will have another level of indentation, so class methods shouldn't be that easy to mistake for instance methods.

I'd recommend adding:

* Alphabetize properties within each CSS rule

To here: https://github.com/styleguide/css

I strongly prefer logically grouping related styles.

The only advantage from alphabetizing rules is perhaps slightly faster scanning of rules, but I don't think it is even very helpful in doing that. You generally don't have that many rules in a single style anyway so it's not a problem that needs solving.

However, by grouping related styles I think there are a some small yet worthwhile advantages. Grouped styles can reveal intention, while alphabetizing does not at all. Grouped styles can also make refactoring quicker and less tedious.

I would agree with that if there were a defined, consistent logic, but I've never seen that done well across multiple developers. After switching to alphabetized rules, my CSS feels much more organized and I'd say is much easier to scan and refactor.

Rules order has meaning in CSS: everything else being equal, the last declared rule has precedence.

  # bad
  email_with_name = user.name + ' <' + user.email + '>'

  # good
  email_with_name = "#{user.name} <#{user.email}>"

  # better
  email_with_name = "%s <%s>" % [user.name, user.email]

Why is the latter better? It seems equally good, but less idiomatic and certainly less obvious to most Ruby programmers.

The structure of strings like

  email_with_name = "%s <%s>" % [user.name, user.email]

can be easily identified even when many fields are added.

On the contrary, strings like

  email_with_name = "#{user.name} <#{user.email}>"

become quite cramped as soon as you use three or more fields.

I'm not sure what you mean by cramped, but I think that the interpolation style is, on the contrary, much easier to read when you have multiple fields:

  puts "#{num} #{vessels} of #{liquid} on the #{where}, you #{verb1} one #{adverb1}, #{verb2} it #{adverb2}, #{num - 1} #{vessels} of #{liquid} on the #{where}"
  puts "%d %s of %s on the %s, you %s one %s, %s it %s, %i %s of %s on the %s", [ num, vessels, liquid, where, verb1, adverb1, verb2, adverb2, num - 1 ]

It's an extreme example, but you just read the first, while you have to think about the second.

I don't get why people are using RGB hex values. I guess all of us find it much easier to read and understand rgb(230, 30, 30) than #e61e1e. And if we don't have to support IE8, I think hsl(0, 80%, 51%) is even better.

Interesting to see their choice in the never ending battle about the indentation of "private" in ruby. The lack of a empty line below "private" seems like it would make the code hard to read. The "private" could easily be missed.

Currently I prefer using "private" indented to the same level as "def", with no change of indentation of code after "private", and an empty line before and after "private".

Ruby doesn't have something like PEP for Python?

Only unofficial

For those of you wondering what this is: http://www.python.org/dev/peps/pep-0008/

It did several years ago but it went by the by. I'm struggling to remember what it was called.. Now, features are raised on the MRI Redmine, such as: https://bugs.ruby-lang.org/issues/6242

What's the reason for recommending 2 space indents ? I find it easier to read with 4 or even more spaces indent.

I was wondering the same thing.

Recently, a coworker and myself were debating the use of tabs vs. spaces - and while I know this is a battle that has been going on for a while and won't end any time soon - one of my points against spaces was due to readability. 2 space soft-tabs seems much harder to follow - while using hard tabs for indentation, you are able to adjust your editor to visually work out what suits you best (2 spaces, 4 spaces or even 8 spaces).

My coworker does some funky blend of spaces and tabs. His tab key inserts 4 spaces, but if he tabs twice (8 characters), it becomes the tab character. He says it works better when printing.

I can't for the life of me figure out how to replicate the effect in vim to be able to read his files logically, and he can't tell me because he's an emacs user.

Something like:

set ts=8 sw=4 et sta

(tabstops to 8 (the default), shift width to 4, expand tabs, smart tabs)

It's the least amount that's least like to be accidental while still visually significant. The emphasis on the least amount is to maintain good line lengths, which can be an issue with the way many Rubyists namespace their code or pull off method chains.

I wonder what is the problem with hard-tabs.

I enjoy 8-space indents, fellow programmers prefer 4-space indents, some others 2-space indents. With hard-tabs each of us is able to use their preferred settings without messing with the commits (setting the tab-to-space ratio in text editor).

They should have an HTML guide which should say: don't write <img> tags which set an image's size to 4x less than its original size, it will look pixelated.

I might be missing something very obvious, but what's the license on this? Specifically, on the CSS buttons and the forms, not just the code formatting.

No mention of the maximum line length in the Ruby style guide. Sigh...

We tend to stay under 80c for everything (minus views, which can be trickier). Probably should add that to the guide.

Second bullet point under Coding Style: "Keep lines fewer than 80 characters."

If you're commenting on the dangers on no semis I would put money on the fact you haven't given it a real try. Perhaps you tried it, had an issue once, and immediately reverted. Its like critiquing a book you haven't read.

After reading this, I just want to say that I do not think semicolon-less javascript is, in general, a good idea.

Development projects are almost always team efforts. And, unfortunately, there's almost always one or two team members who aren't very good. Some of you folks that only work on startups with brilliant people might disagree, but in my experience most development teams have some bad apples who have let their tech skills rot, or won't try for some reason or another.

These people are going to have a tremendous time just writing halfway competent javascript (especially since it's probably not similar to their OOP language of choice). Suggesting they write code in an uncommon way is just begging for trouble. Most of the examples on the internet use semicolons, as do most of the frameworks. In fact, I hope that members of my future teams never see this javascript style guide or even know that it's possible to omit semicolons in many cases.

I'm quite sure someday soon I'm going to hear this over the cube wall: "Hey, GitHub doesn't use semicolons in their javascript so I won't either!" And then I will be very, very sad.

Yeah, you save one character per line, and gain a lot more in cognitive friction. Words to live by: "don't make me think" even about whether or not I need a semi-colon here.

Dropped semicolons about 6 months ago and haven't had a problem even once.

Once you get into the habit, it requires zero extra effort. You just know when to put them in, same way you just know when to use parens vs curlies.

Coding with semis is like coding with parens around every expression; unnecessary and paranoid.

  > Coding with semis is like coding with parens around every expression; 
  > unnecessary and paranoid.

Unless, of course, one is using Scheme or some other Lisp variant.

Good thing it's a Javascript style guide, huh? ;)

It was the "unnecessary and paranoid" bit that really turned me off. Flaming everyone who doesn't use the same style wasn't warranted, IMNSHO. I responded with snark.

I think in general the answer may be to use the style that mimics the other code in the application. Rubyists may prefer the newline endings; those using Java or PHP may find having the semi-colon more comfortable. Having less contextual switch between languages may be easier.

Such a sweeping statement seemed a bit ridiculous. Choosing to interpret it in a broader context was intended to highlight my opinion of the original statement.

EDIT: It's been fun watching the mod of the GP go up and down ... zero, back to one, then to zero ... over and over.

It's unnecessary because apps continue to work with little to no modification without semis, it's paranoid because all of the reasons people come up with for why semicolons are important (lint tools? minifiers? bugs?), are all non-problems if you spend a few minutes changing your habits.

But why? Why drop the semicolons? What is the benefit? If the net result is exactly the same, why try and be tricky? What is the point, other than you can do it. But there are many tricky things we can do with programming languages, but very few we should

He never said the net result was the same.

He said that there were no problems.

No problems doesn't imply no benefits.

On a team of a sufficient size, dropping semicolons hurts. Someone will be running a static analysis "lint" program in their editor, and it will always flag those missing semicolons. That certainly adds additional cognitive friction to those users, who constantly see the reminder to add the missing semicolon.

I guess I've never understood the argument for omitting them. It seems like an unnecessary trick. Coding guidelines should enforce the simplest possible patterns and techniques.

Unlikely the default lint options are going to suit all teams anyway. JSLint/JSHint can, and should be configured for your team's style.

The point is to reduce noise. JavaScript already has a lot of cruft going on with all its anonymous functions, parens and curlies… anything to reduce the signal to noise ratio is a step in the right direction.

If you really cared about that you would use coffee script. Dropping semicolons is such a triviality.

I do use coffeescript on appropriate projects.

That's a good point, even the reference article states that, in some cases, a semicolon is necessary (in the begin of the line). If it's necessary reason about use it on some cases (and mistakes will happen), why not use it on every line?

The guide doesn't have anything about brackets and spacing on function calls (maybe because it's dropping the language).

I completely agree with you. I'm also unsure about imposing coffeescript - although I find it interesting - in an enterprise environment.

I'd rather have nicely crafted and long winded javascript with convention followed, than half javascript and half coffeescript which will hurt people's brain at some stage.

Eventually, that stuff bites you and productivity - all that for the fame of using a brand new language (no offence intended) in the enterprise world. Assuming code is meant to live on for 4-5 years at least here.

And even if your team is superstars now it doesn't mean it always will be. What happens a few years down the road when you now have a team 5 or 10 times the size with people of varying skillsets and areas of expertise. Now that guy with mostly Python or Ruby experience needs to make some changes to your JavaScript code with what little knowledge of the language they have. It's helpful to write things to be as idiomatic as possible in those cases.

Well, the same thing could be said about "that guy with mostly C, PHP or Java experience".

I wasn't singling out any particular languages, just giving an example. It's a lot easier to understand code in an unfamiliar language if it's written in an idiomatic fashion as opposed to using all sorts of tricks.

Reply on news.ycombinator.com