from Hacker News

Just LGTM on Pull Request comments? You're failing as a dev

by baristaGeek on 9/25/23, 8:01 PM with 7 comments

• by jraph on 9/25/23, 8:18 PM

  The article seems to equate lgtm to "not carefully reviewed".

  Where I work, we only open pull requests when we feel the code must be reviewed. When I open a PR, I trust my colleagues to carefully review the code and they make you prove your code is fine if in doubt or let you know if there's something to improve / that could be improved.

  If they write lgtm, they mean it. Lgtm does not mean "I didn't review the changes".

  It's not specific to my team, I've also seen it when contributing to random OSS project.

  I don't want people to try hard to come up with something to say, if it looks good (or even good enough, often) let's just merge and move on.

  Now, sure, if a PR is open, let's carefully review it and comment if needed. But deciding that lgtm is bad seems unwise and looks like cargo culting.

  If someone writes lgtm and did not carefully review / raise something that could be improved, that's the thing to fix. If needed.

  Fix the disease, not a symptom.

  LGTM when things look good makes PRs efficient, it encourages opening PRs and thus getting good reviews when needed.

• by andrewfromx on 9/25/23, 8:05 PM

  Depends. What's the goal? Get the PR merge as quick as possible to keep momentum going on a project or nit pick over details that are "just like your opinion, man" and try for the never ending quest of perfect code.

• by mjoin on 9/25/23, 9:30 PM

  in my experience, technical details have been figured out on a whiteboard before, and then it can happen that a PR is just going to be LGTM and merge. not all conversations happen on a PR system. people talk IRL too

• by brodouevencode on 9/25/23, 8:07 PM

  Perfect is the enemy of good.