932
Looks good to me 👍 (i.ibb.co)
submitted 4 months ago by ZeroCool@feddit.org to c/programmer_humor@programming.dev
26 comments fedilink hide all child comments
all 28 comments
sorted by: hot top controversial new old
[-] souperk@reddthat.com 98 points 4 months ago

I am definitely guilt for that, but I find this approach really productive. We use small bug fixes as an opportunity to improve the code quality. Bigger PRs often introduce new features and take a lot of time, you know the other person is tired and needs to move on, so we focus on the bigger picture, requesting changes only if there is a bug or an important structural issue.

[-] NocturnalMorning@lemmy.world 47 points 4 months ago

I always try to review the code anyway. There's no guarantee that what they wrote is doing what you want it to do. Sometimes I find the person was told to do something and didn't realize it actually needs to do Y and not just X, or visa versa.

[-] ScampiLover@lemmy.world 20 points 4 months ago

I like to shoot for the middle ground: skim for key functions and check those, run code locally to see if it does roughly what I think it should do and if it does merge it into dev and see what breaks.

Small PRs get nitpicked to death since they're almost certainly around more important code

[-] derpgon@programming.dev 10 points 4 months ago

Especially when you see a change in code, but not in tests ☠️

[-] souperk@reddthat.com 2 points 4 months ago

Yes, I always review the code, just avoid nitpicking the hell out of it.

[-] NocturnalMorning@lemmy.world 1 points 4 months ago

Yeah, sorry, totally misread your comment.

[-] breakingcups@lemmy.world 2 points 4 months ago

So you're always behind, patching up small bits of code that don't comply with your guidelines, while letting big changes with, by deduction, worse code quality through?

[-] souperk@reddthat.com 3 points 4 months ago

Not really, we are a small team and we generally trust each other. Sure there are things that could have been better, but it's not bad either.

[-] grrgyle@slrpnk.net 46 points 4 months ago

This and bike shedding.

[-] jubilationtcornpone@sh.itjust.works 42 points 4 months ago

Reviewing large PR's is hard. Breaking apart large PR's that are all related changes into smaller PR's is also hard.

If I submit a big one, I usually leave notes in the description explaining where the "core" changes are and what they are trying to accomplish. The goal being to give the reviewers a good starting point.

I also like to unit test the shit out of my code which helps a lot. The main issue there is getting management to embrace unit tests. Unit tests often double the effort up front but save tons of time in the long run. We're going to spend the time one way or the other. Better to do it up front when it's "cheaper" because charging it to the tech debt credit card racks up lots of expensive interest.

[-] zalgotext@sh.itjust.works 13 points 4 months ago

I can't believe we still have to justify writing unit tests to management in the year 2024

[-] pageflight@lemmy.world 11 points 4 months ago

Yeah, if you don't want the next dev (or your future self) to accidentally undo that corner case you fixed, better put a unit test on it.

[-] TheSlad@sh.itjust.works 36 points 4 months ago

In my first programming job, I would actually do code reviews by pausing my own work, pulling their branch and building it locally, then using debug mode to step through every changed or added line of code looking for bugs, unaccounted for edge cases, and code quality issues.

...I dont do that anymore, I now go "looks good to me" even on 10 line reviews.

[-] silasmariner@programming.dev 2 points 4 months ago

Yeah but I bet you do it sometimes on your own pull requests even after you've opened them don't you?

[-] gnutrino@programming.dev 19 points 4 months ago

This is why I always rename all the variables in the project on each PR.

[-] jol@discuss.tchncs.de 15 points 4 months ago

I know this is a joke, but it you did that I would reject the pr with the reason of too many things at once. Reopen separate PR to refactor variable names. I actually constaly get people doing this and it's dangerous exactly for the reason you're joking about. Makes it easier for errors to slip in.

[-] silasmariner@programming.dev 2 points 4 months ago

I know you're playing the straight man to a joke, but actually you can apply a linter, then tell GitHub to ignore the implied ownership history for the purposes of blame from that reclining pr. All such prs are massive and yet by virtue of the replayability of the linter it's also very easy to ensure errors didn't slip in when reviewing.

I know the original comment was about renaming all the variables, but that's obviously deliberately absurd, so I'm using here a completely realistic example instead.

[-] Lifter@discuss.tchncs.de 0 points 4 months ago

This will lead to change fatigue. People will rather not cleanup as they go anymore and just get the work done, with worse and worse code quality as a result.

[-] jol@discuss.tchncs.de 1 points 4 months ago

I prefer that than to sneak defects in huge PRs.

[-] jack@monero.town 16 points 4 months ago

Ask him to do 500 lines and he will never look at it, making you wait forever

[-] Honytawk@lemmy.zip 21 points 4 months ago

Meanwhile, ask a c-suite to do 500 lines, and they party until they overdose.

[-] brrt@sh.itjust.works 12 points 4 months ago

Just give them 10 lines at a time from the 500 lines one. Is this how micromanagement was born?

[-] MonkderDritte@feddit.de 4 points 4 months ago

Pycharms warns me about cognitive complexity of functions. Other IDEs too i assume?

[-] Bonje@lemmy.world 1 points 4 months ago

The trick is that 10 lines of code usually pull in thousands as they are likely function calls.

[-] magic_lobster_party@kbin.run 0 points 4 months ago

I’m the opposite. If there’s 500 lines I will look closer for issues. If there’s only 10 lines it’s LGTM. I’m not going to reward such behavior.

this post was submitted on 02 Jul 2024
932 points (98.5% liked)

Programmer Humor

19623 readers
1 users here now

Welcome to Programmer Humor!

This is a place where you can post jokes, memes, humor, etc. related to programming!

For sharing awful code theres also Programming Horror.

Rules

founded 1 year ago
MODERATORS
Feyter@programming.dev
anzo@programming.dev
BurningTurtle@programming.dev
pylapp@programming.dev