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.
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.
What we argue is that code review's goal is not only preventing the deployment of bad code to production (very important by itself) but also osmote information about the codebase to everyone on the team.
yeah but how do you prevent egos and personalities to dominate and have it not turn into "well bob said do it this way and I'm in bob's camp." "Oh yeah, bob doesn't know what he's talking about..." not the mention the subtle stuff going on behind the scenes not talked about. Some people are friends outside of work and are having a fight that has nothing to do with the code but taking it out on each other in the PR! https://www.stilldrinking.org/programming-sucks
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
We completely agree with this! It's better to say, "It's imperfect due to X reason, but let's merge and fix it later," rather than just a "lgtm" message that doesn't educate anyone."
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.