DEV Community

Eric Meadows
Eric Meadows

Posted on

LGTM = Scary

LGTM - the scariest words to see as a CTO. That means devs didn't spend enough time on a PR.

Latest comments (20)

Collapse
 
wadecodez profile image
Wade Zimmerman

code review looks different in different stages of project's lifetime, and it also depends on how you deploy code. Sometimes teams have staging servers, automated testing, and dedicated QA teams.

Code review is more about skimming the difference in code and checking that the basic requirements were met. Does the code actually run/compile? Did someone lint the entire project? Did they branch off of the right features? Do the automated tests pass?

Looks Good To Me (LGTM) is perfectly valid.

Collapse
 
maroun_baydoun profile image
Maroun Baydoun

Thinking that PRs create a better codebase = Scary

Collapse
 
apetryla profile image
Aidas Petryla

Two sentence posts = Vague

I don't really understand Your point. From the information You provided, which is "LGTM = Scary" I have no context whatsoever on what You're trying to say.

LGTM can be after long debates and many comments on the MR (PR).

LGTM can be on short changes, which simply look good.

LGTM can be anything.

I'm curious in what You wanted to say, however, I don't feel that I really get it.

Collapse
 
ant_f_dev profile image
Anthony Fung

What would be appropriate if it genuinely doesn't look like there's anything wrong with it?

Collapse
 
joelbonetr profile image
JoelBonetR πŸ₯‡

"Approves PR without comments" πŸ˜‚πŸ˜‚

Collapse
 
ant_f_dev profile image
Anthony Fung

Nice!

On the subject of acronyms, I have a short story. I once had a colleague who used to ask quite a lot of questions on how to do things.

Let me be clear: there's absolutely nothing wrong with asking questions whenever you're unsure - I do this all the time.

It was a Friday afternoon one day, and the aforementioned colleague asked his project lead how to do something. As a joke, the project lead sent him a link to lmgtfy (let me google that for you). Unfortunately, he didn't see the funny side and stormed out of the office in a rage.

Thread Thread
 
joelbonetr profile image
JoelBonetR πŸ₯‡ • Edited

Well... one must be consistent with their actions.

Googling things is a basic skill and does not waste teammate's time (plus chat GPT also can help a lot those days), asking things is OK but pretending others to take time to answer everything without putting the effort on trying to solve them by yourself is impolite and selfish, at least.

I would say that there are questions (reasonable, this person wants to learn more, he/she is putting effort...) and "questions" (this bad boy didn't even check the reference/doc, he/she is just interested on fixing an issue right now as fast as possible -they want you to fix it with your time- and move on).

All of us can be in the "questions" side sometimes but whenever this manners keep showing up as the norm... β›³

Thread Thread
 
ant_f_dev profile image
Anthony Fung

Yes, fair point - at least try before asking.

I can't remember how far into that project it was when it happened, but he was learning React/Redux for the first time. Let's all try to keep in mind that different people learn in different ways and at different rates.

I do understand what you're saying though. And I also understand that having to context-switch repeatedly is very damaging for productivity.

Thread Thread
 
joelbonetr profile image
JoelBonetR πŸ₯‡

Totally agree!

Collapse
 
jmfayard profile image
Jean-Michel (agent double)

Counterargument: it's probably worse if the devs missed the one important thing in the PR but did a lots of nitpicking that give the feeling of a productive code review.

Collapse
 
canro91 profile image
Cesar Aguirre

If a PR is short and focused and anyone can review it under 10 minutes, a "LGTM" LGMT :)

Collapse
 
mellen profile image
Matt Ellen

Large Ghost Through Mirror?

Collapse
 
joelbonetr profile image
JoelBonetR πŸ₯‡

Collapse
 
mellen profile image
Matt Ellen

Ah, yes, the Internet is both the large ghost and the mirror.

Very deep.

Collapse
 
trpricesoftware profile image
Taylor R Price

And "LGTM = Scary" is a great headline/sound bite but doesn't encapsulate the entire issue.

If the merge is a small issue then LGTM is likely fine.

Are there other comments on the PR?

Do you have a different review tool? We use Fisheye/Crucible instead of reviewing via PR. How do you know it wasn't reviewed?

Collapse
 
rcls profile image
OssiDev

Yup. That's why I have my PRs still waiting in the review queue after three days. I don't know which is worse.

Collapse
 
joelbonetr profile image
JoelBonetR πŸ₯‡

5 days have passed so far, how is it going? 😁

Collapse
 
manojlingala profile image
manojlingala

LGTM , best way to cut down the discussion 😁

Collapse
 
360macky profile image
Marcelo Arias

I find the direct merge without a message more scary πŸ˜….

Collapse
 
alexkipen profile image
Alex Kipen

?