One of the most salient features of our Tech Hiring culture is that there is so much bullshit. Everyone knows this. Each of us contributes his share. But we tend to take the situation for granted.
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.
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.
Experienced software engineer specialized in event-driven microservices & cloud-native SaaS apps. Let's connect to drive digital transformation in your org.
My name is Taylor Price. I am a Software Engineer (C, C++, C#, and more), Consultant, Advocate for smart code and good testing, Owner/Operator of Taylor Built Solutions
Tech Lead/Team Lead. Senior WebDev.
Intermediate Grade on Computer Systems-
High Grade on Web Application Development-
MBA (+Marketing+HHRR).
Studied a bit of law, economics and design
Location
Spain
Education
Higher Level Education Certificate on Web Application Development
Tech Lead/Team Lead. Senior WebDev.
Intermediate Grade on Computer Systems-
High Grade on Web Application Development-
MBA (+Marketing+HHRR).
Studied a bit of law, economics and design
Location
Spain
Education
Higher Level Education Certificate on Web Application Development
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.
Tech Lead/Team Lead. Senior WebDev.
Intermediate Grade on Computer Systems-
High Grade on Web Application Development-
MBA (+Marketing+HHRR).
Studied a bit of law, economics and design
Location
Spain
Education
Higher Level Education Certificate on Web Application Development
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... ⛳
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.
Tech Lead/Team Lead. Senior WebDev.
Intermediate Grade on Computer Systems-
High Grade on Web Application Development-
MBA (+Marketing+HHRR).
Studied a bit of law, economics and design
Location
Spain
Education
Higher Level Education Certificate on Web Application Development
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?
Software engineer, architect, consultant, amateur UI/UX designer, computer enthusiast, gamer. Often coming up with ideas and thoughts that I then write into a post within the next 60 minutes.
Tech Lead/Team Lead. Senior WebDev.
Intermediate Grade on Computer Systems-
High Grade on Web Application Development-
MBA (+Marketing+HHRR).
Studied a bit of law, economics and design
Location
Spain
Education
Higher Level Education Certificate on Web Application Development
Software engineer, lifelong learner, language enthusiast & avid reader — Get my free 7-day email course to refactor your coding career: bit.ly/csarag-lessons
Location
Colombia 🇨🇴 (not Columbia)
Work
Content, Courses & Training for .NET teams — Helping teams to write maintainable & performant code
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.
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 😁
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.
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!
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.
Thinking that PRs create a better codebase = Scary
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? 😁
If a PR is short and focused and anyone can review it under 10 minutes, a "LGTM" LGMT :)