commits-pr-and-you



commits-pr-and-you

0 0


commits-pr-and-you

Une petite presentation on commits and pull reviews

On Github pjaspers / commits-pr-and-you

Commits, PRs, Code Reviews and You.

Some terminology

Dumb < Past You < Present You < Future You < Not Dumb

Commits

Commits are written by Present You, they will be usable to Future You, it's important to include as much context as possible to avoid Future You thinking of Past You as an idiot.

Some examples

Fixes thing

This makes Bob sad.

              
Fix: User could not signup because button was gone

If a user tried to sign up on a mobile devices, the button would not
be displayed. This was caused by the `float: left` on the button
itself, by removing this it is working again on all platforms.

[fixes #123456]
              
            

This makes Bob happy.

              Committed all the things
            

Stop making Bob sad.

              
Committed all the things...

This is a big commit, sorry. The things that I've tried to do in this commit:

- Fix bug with the thingamajig
- Enhance the AI
- Turned off various logging things
              
            

Let's make Bob happy!

Commits are a diary

(Explain what and why)

Comments

Comments also make 'Past You' look way smarter than he actually was.

              
def word_list
  return @word_list if @word_list
  indexes = (1..6).to_a
     .repeated_permutation(5).map(&:join)
     .first(valid_words.length)
  @word_list = Hash[indexes.zip(valid_words)]
  @word_list
end
              
            

Versus

              
def word_list
  return @word_list if @word_list
  # The coolest line in this whole source.
  #
  # Get all permuations with `1,2,3,4,5,6` (e.g. from '11111' 66666')
  # only take as many as we need.
  indexes = (1..6).to_a.repeated_permutation(5)
    .map(&:join).valid_words.length)
   # Zip the permutations with the words
  # So this creates a hash like:
  #
  #      {'11111': 'first',
  #       ...
  #       '66665': 'another',
  #       '66666': 'last'}
   @word_list = Hash[indexes.zip(valid_words)]
  @word_list
end
              
            

Bad code with comments is less bad than bad code.

Smart code without comments is bad code.

Bad code with comments != smart code

Pull Requests

The perfect Pull Request

  • Tells what should happen (Context)
  • Makes it easy to review
  • High level overview
  • Has test to make sure it's solved
  • Tells why you did it this way

Code reviews

Some guidelines

Don't be a dick Don't be afraid to ask a question Don't tell, ask. (See 1) Even a +1 is useful

You are not your code

CI

It's a PITA

Sometimes you need pita.

Beste remedie tegen ‘goesting in pita’ is pita. Net gebeld. 🐣

@atog

You

  • If you want something reviewed by someone, ping them on Slack.
  • If you want something reviewed ping on #dev
  • If you have some time, look/ask for something to review.
  • Enter in Freckle on the project with tag #qa

Questions?