maindrop-00113 lines mergedscoreboardmade to order · ships in about a week
vibetokens

50 jokes · code review · all original

Funny Code Review Comments: LGTM, Nits and Worse

The funniest code review comments are the true ones: an LGTM posted four minutes after a 2,000-line diff, or fourteen nits about naming followed by an approval. Here are 50 we wrote, sorted by what the reviewer was doing at the time, and each one is a comment you have left or received.

Code review is where a team says what it thinks, in writing, with a timestamp. That makes it the most quotable part of the job. Every comment on this page is original. None is copied from a real pull request, though most of them have happened somewhere this week, probably twice.

The sections follow a review from the top: the approval, the nits, the red badge, the long wait and the thread that grows underneath it all. After each one there is a piece that says the same thing in fewer words, printed to order.

If a term is unfamiliar, the developer dictionary has the plain definition. If a comment is familiar, that is the point.

01 The approval · 10 jokes

LGTM, and what it covered

LGTM means “looks good to me.” In theory it means the reviewer read the change and has no objections. In practice there are several other theories, and all of them have been approved.

  1. 01LGTM. I trust you, and also the diff would not load.
  2. 02LGTM. Reviewed on my phone in the elevator, somewhere between floors three and nine.
  3. 03Approved 41 seconds after the request. 2,400 lines. A personal record for both of us.
  4. 04LGTM, assuming the skipped tests would pass if anyone ran them.
  5. 05Looks good to me. I did not say it looks good to anyone else.
  6. 06LGTM. I have no idea what this does, but it does it with very consistent indentation.
  7. 07Approving to clear my notifications. That is not a review strategy. It is a mood.
  8. 08LGTM for the parts I understood, which were the comments.
  9. 09LGTM. The description says “small refactor” and I have chosen to believe it.
  10. 10Approved. The file is called legacy_handler_old_2 and I have decided it is not my business.

02 Nits · 10 jokes

Nit: fourteen small comments

A nit is a small comment that does not block the merge: a name, a blank line, a word in a docstring. One nit is helpful. Fourteen is a personality, and an approval with nits attached is still an approval, technically.

  1. 11nit: this variable is named data. So is the one above it. So is the function.
  2. 12nit: trailing whitespace on line 212. I am not upset. I am just pointing at it.
  3. 13nit: could this be a const? Not a blocker. I will ask again in your next pull request, and the one after that.
  4. 14nit (non-blocking): the whole approach. Approving anyway.
  5. 15Approved with nits. There are 31 nits. Please read them as warmly as they were written, which was at midnight.
  6. 16nit: the word temp has been in the name of this file since 2022.
  7. 17nit: the comment says “quick hack, will clean up.” I would like to discuss the word quick, and the word will.
  8. 18nit: this works, but in a way that makes me feel something.
  9. 19nit: two blank lines here. The style guide says one. The style guide was written by me, in this comment.
  10. 20Small nit, feel free to ignore: please do not ignore it.

03 The red badge · 10 jokes

Changes requested, politely

Changes requested is the red badge on a pull request. With required reviews turned on, it blocks the merge until that reviewer comes back, so the wording around it matters more than anything else in the thread.

  1. 21Requesting changes. Just one, but it is the whole file.
  2. 22Could you walk me through your thinking here? I have read it four times and I think it is walking me.
  3. 23This is clever. Please make it less clever before someone has to read it at 2 AM.
  4. 24Have you considered not doing this?
  5. 25I left a few comments. GitHub says it is 46. I am choosing to stand by the word few.
  6. 26Blocking on one thing: the function called doEverything currently does everything.
  7. 27Can we add a test for this? Not because I doubt you. Because I doubt the future.
  8. 28This loop runs once per row. There are four million rows. I have done the math so you do not have to.
  9. 29Why is this in a try/catch that catches the error and then does nothing with it? Asking for the error.
  10. 30Changes requested, mostly to the pull request description, which is the single word “stuff.”

04 The wait · 10 jokes

Awaiting review, still

Awaiting review is the state between opening a pull request and anyone caring about it. It is the slowest part of shipping, and the comments it produces are the most patient writing in software.

  1. 31Gentle bump. This was opened before the office changed coffee vendors.
  2. 32Bumping this to the top of your queue, where it can wait with more visibility.
  3. 33Re-requested review. Also asked in Slack. Also mentioned it at standup, in a tone.
  4. 34PTAL. I have rebased it twice while waiting, so technically it is a new pull request every morning.
  5. 35Any eyes on this? One eye is fine. A glance would be generous.
  6. 36Still awaiting review. The branch has now outlived two sprints and a reorg.
  7. 37Added three reviewers so one of them would feel responsible. So far, none of them do.
  8. 38No rush on this. (Posted the afternoon before the release.)
  9. 39The CODEOWNERS file says you own this folder. I have come to collect.
  10. 40Pinging again in case the first four pings were a network issue.

05 The thread · 10 jokes

The rest of the thread

Some of the best review comments are not about the code at all. They happen three replies down, after everyone has forgotten which line started it, and they are usually typed with great care by people who would rather be merging.

  1. 41Per my last review comment, which was also about this.
  2. 42Resolved the conversation. Did not resolve the issue. These are two different buttons.
  3. 43Happy to hop on a quick call. Nobody in this thread believes the word quick.
  4. 44Marked as outdated by GitHub. Not by me. I still feel this way.
  5. 45“Let's take this offline,” wrote reply number 38.
  6. 46Great catch. That was my code. Please do not run git blame on the rest of this file.
  7. 47Suggested change: deletes the entire function. Click commit suggestion to accept, if you are feeling brave.
  8. 48Not blocking, but I wrote 900 words about it, so you can decide how much it is not blocking.
  9. 49Thanks for the review. Addressed everything except the comments I disagreed with, which I addressed emotionally.
  10. 50+1 to everything the previous reviewer said. I did not read what the previous reviewer said.

How to leave a review comment people will act on

Say what you saw, then why it matters. “This query runs once per row” is something an author can fix. “This is slow” is something an author can only feel.

Separate what blocks the merge from what you would have done differently. Prefix the second kind with nit: so nobody has to guess. If the same nit comes up every week, it belongs in a linter or formatter rule, not in a person.

When you approve, say what you checked. “LGTM, ran the migration up and down locally” is worth more than a thumbs-up, and it is the opposite of the first section of this page.

Ask questions when you are unsure, and keep the comment on the code rather than on the author. Most review threads that go wrong do it in the third reply, not the first.

And review promptly. A fast, careful review is worth more than a slow, perfect one, and a pull request that waits a week collects exactly the comments in section four.

If one of these is your team's entire review culture, it comes printed. The gift guide for programmers sorts every line by budget, and the dictionary explains the ones your manager asks about.

06 The plain answers

Questions.

What does LGTM mean in a code review?
LGTM stands for “looks good to me.” A reviewer writes it to approve a pull request and signal that it is ready to merge. A useful LGTM also says what was checked, like “ran the migration locally.” LGTM in the dictionary →
What is a nit in code review?
A nit, short for nitpick, is a small comment that does not block the merge: a variable name, a typo, a style preference. “Approved with nits” means the change can merge, and the author may take or skip the small suggestions. Nit in the dictionary →
What does “changes requested” mean on GitHub?
It is a review result that asks the author to change specific things before the pull request merges. When the repository requires reviews, it blocks the merge until that reviewer approves a later version or the review is dismissed.
How do you write a code review comment that is not rude?
Say what you saw and why it matters, ask questions instead of issuing verdicts, and label small preferences as nits so the author knows what actually blocks the merge. Comment on the code, not the person.

More jokes

Keep reading.