LGTM - the scariest words to see as a CTO. That means devs didn't spend enough time on a PR.
LGTM - the scariest words to see as a CTO. That means devs didn't spend enough time on a PR.
For further actions, you may consider blocking this person and/or reporting abuse
I find the direct merge without a message more scary 😅.
?
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.
What would be appropriate if it genuinely doesn't look like there's anything wrong with it?
"Approves PR without comments" 😂😂
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.
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... ⛳
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.
Totally agree!
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?
Large Ghost Through Mirror?
Ah, yes, the Internet is both the large ghost and the mirror.
Very deep.
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.
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.
LGTM , best way to cut down the discussion 😁
Yup. That's why I have my PRs still waiting in the review queue after three days. I don't know which is worse.
5 days have passed so far, how is it going? 😁
Thinking that PRs create a better codebase = Scary
If a PR is short and focused and anyone can review it under 10 minutes, a "LGTM" LGMT :)