This is Style Guide. It was inspired by Airbnb's guide
- Pull Requests
- Commenting 1. File/class-level comments 1. Function comments 1. Block and inline comments 1. Punctuation, spelling, and grammar 1. TODO comments 1. Commented-out code
- Whitespace 1. Indentation 1. Inline 1. Newlines
- Line Length
- API Documentation
What a Pull Request should include?
A pull request at mywebsite should always include the following:
- Descriptive title of the fix or feature.
- Necessary information in the Pull Request description.
- [ticket_link] to the related issue in the project management system.
- Any additional information that may be of help to the reviewer.
- Code required by the feature
- Tests covering the feature on a necessary level (a spacing issue PR etc. do not require tests)
- Necessary commenting on the feature, see Commenting for more information
- A Pull Request including changes to the API or new content to it should always include edits to API documentation
A short one sentence description of the change (#ticket_number).
If a longer description is needed, it will appear here. If not, this line won't appear.
* bullets of important changes.
* unrelated to the main commit message.
Merges ##{pull_request_number}
LGTM given by: @god, @otherReviewerName
The [ticket_link] provides detail on what ticket the commit is related to inside of our project management system (JIRA or Trello).
Squash merging ensures that every change is shown in full context of the delivered feature, so if you do git-blame on a line, you will see the full feature that introduced the change.
Though a pain to write, comments are absolutely vital to keeping our code readable. The following rules describe what you should comment and where. But remember: while comments are very important, the best code is self-documenting. Giving sensible names to types and variables is much better than using obscure names that you must then explain through comments.
When writing your comments, write for your audience: the next contributor who will need to understand your code. Be generous — the next one may be you!
Portions of this section borrow heavily from the AirBNB, Google C++ and Python style guides.
Every class definition should have an accompanying comment that describes what it is for and how it should be used.
A file that contains zero classes or more than one class should have a comment at the top describing its contents.
# Automatic conversion of one locale to another where it is possible, like
# American to British English.
module Translation
# Class for converting between text between similar locales.
# Right now only conversion between American English -> British, Canadian,
# Australian, New Zealand variations is provided.
class PrimAndProper
def initialize
@converters = { :en => { :"en-AU" => AmericanToAustralian.new,
:"en-CA" => AmericanToCanadian.new,
:"en-GB" => AmericanToBritish.new,
:"en-NZ" => AmericanToKiwi.new,
} }
end
...
# Applies transforms to American English that are common to
# variants of all other English colonies.
class AmericanToColonial
...
end
# Converts American to British English.
# In addition to general Colonial English variations, changes "apartment"
# to "flat".
class AmericanToBritish < AmericanToColonial
...
endAll files, including data and config files, should have file-level comments.
# List of American-to-British spelling variants.
#
# This list is made with
# lib/tasks/list_american_to_british_spelling_variants.rake.
#
# It contains words with general spelling variation patterns:
# [trave]led/lled, [real]ize/ise, [flav]or/our, [cent]er/re, plus
# and these extras:
# learned/learnt, practices/practises, airplane/aeroplane, ...
sectarianizes: sectarianises
neutralization: neutralisation
...Every function declaration should have comments immediately preceding it that describe what the function does and how to use it. These comments should be descriptive ("Opens the file") rather than imperative ("Open the file"); the comment describes the function, it does not tell the function what to do. In general, these comments do not describe how the function performs its task. Instead, that should be left to comments interspersed in the function's code.
Every function should mention what the inputs and outputs are, unless it meets all of the following criteria:
- not externally visible
- very short
- obvious
You may use whatever format you wish. In Ruby, two popular function documentation schemes are TomDoc and YARD. You can also just write things out concisely:
# Returns the fallback locales for the_locale.
# If opts[:exclude_default] is set, the default locale, which is otherwise
# always the last one in the returned list, will be excluded.
#
# For example:
# fallbacks_for(:"pt-BR")
# => [:"pt-BR", :pt, :en]
# fallbacks_for(:"pt-BR", :exclude_default => true)
# => [:"pt-BR", :pt]
def fallbacks_for(the_locale, opts = {})
...
endThe final place to have comments is in tricky parts of the code. If you're going to have to explain it at the next code review, you should comment it now. Complicated operations get a few lines of comments before the operations commence. Non-obvious ones get comments at the end of the line.
def fallbacks_for(the_locale, opts = {})
# dup() to produce an array that we can mutate.
ret = @fallbacks[the_locale].dup
# We make two assumptions here:
# 1) There is only one default locale (that is, it has no less-specific
# children).
# 2) The default locale is just a language. (Like :en, and not :"en-US".)
if opts[:exclude_default] &&
ret.last == default_locale &&
ret.last != language_from_locale(the_locale)
ret.pop
end
ret
endOn the other hand, never describe the code. Assume the person reading the code knows the language (though not what you're trying to do) better than you do.
Related: do not use block comments. They cannot be preceded by whitespace and are not as easy to spot as regular comments. [link]
# bad
=begin
comment line
another comment line
=end
# good
# comment line
# another comment linePay attention to punctuation, spelling, and grammar; it is easier to read well-written comments than badly written ones.
Comments should be as readable as narrative text, with proper capitalization and punctuation. In many cases, complete sentences are more readable than sentence fragments. Shorter comments, such as comments at the end of a line of code, can sometimes be less formal, but you should be consistent with your style.
Although it can be frustrating to have a code reviewer point out that you are using a comma when you should be using a semicolon, it is very important that source code maintain a high level of clarity and readability. Proper punctuation, spelling, and grammar help with that goal.
Use TODO comments for code that is temporary, a short-term solution, or good-enough but not perfect.
TODOs should include the string TODO in all caps, followed by the full name of the person who can best provide context about the problem referenced by the TODO, in parentheses. A colon is optional. A comment explaining what there is to do is required. The main purpose is to have a consistent TODO format that can be searched to find the person who can provide more details upon request. A TODO is not a commitment that the person referenced will fix the problem. Thus when you create a TODO, it is almost always your name that is given.
# bad
# TODO(RS): Use proper namespacing for this constant.
# bad
# TODO(drumm3rz4lyfe): Use proper namespacing for this constant.
# good
# TODO(Ringo Starr): Use proper namespacing for this constant.- Never leave commented-out code in our codebase. [link]
There are cases in our old codebase where this exists, and is being cleaned up.
-
Use soft-tabs with a two space-indent.[link]
-
Indent
whenas deep ascase. [link]case when song.name == 'Misty' puts 'Not again!' when song.duration > 120 puts 'Too long!' when Time.now.hour > 21 puts "It's too late" else song.play end kind = case year when 1850..1889 then 'Blues' when 1890..1909 then 'Ragtime' when 1910..1929 then 'New Orleans Jazz' when 1930..1939 then 'Swing' when 1940..1950 then 'Bebop' else 'Jazz' end
-
Align function parameters either all on the same line or one per line.[link]
# bad def self.create_translation(phrase_id, phrase_key, target_locale, value, user_id, do_xss_check, allow_verification) ... end # good def self.create_translation(phrase_id, phrase_key, target_locale, value, user_id, do_xss_check, allow_verification) ... end # good def self.create_translation( phrase_id, phrase_key, target_locale, value, user_id, do_xss_check, allow_verification ) ... end
-
Indent succeeding lines in multi-line boolean expressions.[link]
# bad def is_eligible?(user) Trebuchet.current.launch?(ProgramEligibilityHelper::PROGRAM_TREBUCHET_FLAG) && is_in_program?(user) && program_not_expired end # good def is_eligible?(user) Trebuchet.current.launch?(ProgramEligibilityHelper::PROGRAM_TREBUCHET_FLAG) && is_in_program?(user) && program_not_expired end
-
Never leave trailing whitespace. [link]
-
When making inline comments, include a space between the end of the code and the start of your comment. [link]
# bad result = func(a, b)# we might want to change b to c # good result = func(a, b) # we might want to change b to c
-
Use spaces around operators; after commas, colons, and semicolons; and around
{and before}. [link]sum = 1 + 2 a, b = 1, 2 1 > 2 ? true : false; puts 'Hi' [1, 2, 3].each { |e| puts e }
-
Never include a space before a comma. [link]
result = func(a, b)
-
Do not include space inside block parameter pipes. Include one space between parameters in a block. Include one space outside block parameter pipes. [link]
# bad {}.each { | x, y |puts x } # good {}.each { |x, y| puts x }
-
Do not leave space between
!and its argument.[link]!something -
No spaces after
(,[or before],). [link]some(arg).other [1, 2, 3].length
-
Omit whitespace when doing string interpolation.[link]
# bad var = "This #{ foobar } is interpolated." # good var = "This #{foobar} is interpolated."
-
Don't use extra whitespace in range literals.[link]
# bad (0 ... coll).each do |item| # good (0...coll).each do |item|
-
Add a new line after
ifconditions span multiple lines to help differentiate between the conditions and the body. [link]if @reservation_alteration.checkin == @reservation.start_date && @reservation_alteration.checkout == (@reservation.start_date + @reservation.nights) redirect_to_alteration @reservation_alteration end
-
Add a new line after conditionals, blocks, case statements, etc.[link]
if robot.is_awesome? send_robot_present end robot.add_trait(:human_like_intelligence)
-
Don’t include newlines between areas of different indentation (such as around class or module bodies). [link]
# bad class Foo def bar # body omitted end end # good class Foo def bar # body omitted end end
-
Include one, but no more than one, new line between methods.[link]
def a end def b end
-
Use a single empty line to break between statements to break up methods into logical paragraphs internally. [link]
def transformorize_car car = manufacture(options) t = transformer(robot, disguise) car.after_market_mod! t.transform(car) car.assign_cool_name! fleet.add(car) car end
-
End each file with a newline. Don't include multiple newlines at the end of a file. [link]
Keeping code visually grouped together (as a 100-character line limit enforces) makes it easier to understand. For example, you don't have to scroll back and forth on one line to see what's going on -- you can view it all together.
Here are examples from our codebase showing several techniques for breaking complex statements into multiple lines that are all < 100 characters. Notice techniques like:
- liberal use of linebreaks inside unclosed
({[ - chaining methods, ending unfinished chains with a
. - composing long strings by putting strings next to each other, separated by a backslash-then-newline.
- breaking long logical statements with linebreaks after operators like
&&and||
scope = Translation::Phrase.includes(:phrase_translations).
joins(:phrase_screenshots).
where(:phrase_screenshots => {
:controller => controller_name,
:action => JAROMIR_JAGR_SALUTE,
})translation = FactoryGirl.create(
:phrase_translation,
:locale => :is,
:phrase => phrase,
:key => 'phone_number_not_revealed_time_zone',
:value => 'Símanúmerið þitt verður ekki birt. Það er aðeins hægt að hringja á '\
'milli 9:00 og 21:00 %{time_zone}.'
)if @reservation_alteration.checkin == @reservation.start_date &&
@reservation_alteration.checkout == (@reservation.start_date + @reservation.nights)
redirect_to_alteration @reservation_alteration
end<% if @presenter.guest_visa_russia? %>
<%= icon_tile_for(I18n.t("email.reservation_confirmed_guest.visa.details_header",
:default => "Visa for foreign Travelers"),
:beveled_big_icon => "stamp") do %>
<%= I18n.t("email.reservation_confirmed_guest.visa.russia.details_copy",
:default => "Foreign guests travelling to Russia may need to obtain a visa...") %>
<% end %>
<% end %>These code snippets are very much more readable than the alternative:
scope = Translation::Phrase.includes(:phrase_translations).joins(:phrase_screenshots).where(:phrase_screenshots => { :controller => controller_name, :action => JAROMIR_JAGR_SALUTE })
translation = FactoryGirl.create(:phrase_translation, :locale => :is, :phrase => phrase, :key => 'phone_number_not_revealed_time_zone', :value => 'Símanúmerið þitt verður ekki birt. Það er aðeins hægt að hringja á milli 9:00 og 21:00 %{time_zone}.')
if @reservation_alteration.checkin == @reservation.start_date && @reservation_alteration.checkout == (@reservation.start_date + @reservation.nights)
redirect_to_alteration @reservation_alteration
end<% if @presenter.guest_visa_russia? %>
<%= icon_tile_for(I18n.t("email.reservation_confirmed_guest.visa.details_header", :default => "Visa for foreign Travelers"), :beveled_big_icon => "stamp") do %>
<%= I18n.t("email.reservation_confirmed_guest.visa.russia.details_copy", :default => "Foreign guests travelling to Russia may need to obtain a visa prior to...") %>
<% end %>
<% end %>Our API Documentation is located here.