1079
Approved! (lemmy.fmhy.ml)
top 49 comments
sorted by: hot top controversial new old
[-] MurdoMaclachlan@lemmy.world 56 points 1 year ago* (last edited 1 year ago)

Image Transcription: Twitter


Giray Özil, @girayozil

Ask a programmer to review 10 lines of code, he'll find 10 issues. Ask him to do 500 lines and he'll say it looks good.


I am a human who transcribes posts to improve accessibility on Lemmy. Transcriptions help people who use screen readers or other assistive technology to use the site. For more information, see here.

[-] Zana@startrek.website 10 points 1 year ago
[-] beigeoat@lemmy.zip 9 points 1 year ago* (last edited 1 year ago)
[-] rhpp@programming.dev 1 points 1 year ago

I wonder why the downvote counter went into the negatives. (and for some reason that only seems to show in my home instance)

[-] jmk1ng@programming.dev 49 points 1 year ago
[-] Rob@lemmy.world 47 points 1 year ago

Let’s Gamble, Try Merging!

[-] TheGreenGolem@lemm.ee 13 points 1 year ago

Why. Whyyyyyy people need to comment this always? Why isn't just the Approve button enough? I so much hate it.

[-] qwop@programming.dev 18 points 1 year ago* (last edited 1 year ago)

Ah, that's too boring. I have a range of responses to pick from to keep things interesting:

  • LGTM
  • Nice
  • Looks good
  • Thanks
  • Looks great
  • :thumbsup:
  • Looks good to me
  • :shipit:

For me, no text means "I haven't really reviewed this properly so don't want to write anything that could be used against me if (when?) this breaks something in prod"

[-] ironcrotch@aussie.zone 1 points 1 year ago
[-] BeigeAgenda@lemmy.ca 1 points 1 year ago

I reserve "ship it" to mostly trivial bug fixes.

[-] HorseWife@midwest.social 12 points 1 year ago

If you're in a place with codebase analytics you want to have at least one comment on every MR - otherwise the system will start to think you're falling behind... I hate codebase analytics.

[-] IWriteDaCode@programming.dev 6 points 1 year ago

Analytics software like that has made my professional life so annoying at many times.

[-] GTG3000@programming.dev 1 points 1 year ago

In my experience, the managers get confused when issues/PRs are closed without any comment.
Useless comments beat having them pop into your slack to ask "hey, did you review this?" with a link to an approved PR.

[-] fusio@lemmy.world 36 points 1 year ago

That's why PR should be small. It's much better to have multiple PRs than a single big one.

Totally fair to have gigantic PR full of boilerplate code, but generally you can split the boilerplate and your feature in 2 PRs, where only the feature will get a proper review.

All of this obviously depends on the criticality of the system :p

[-] Rikudou_Sage@lemmy.world 10 points 1 year ago

Or split them per commit at the minimum if you don't want to create a separate PR.

[-] Asifall@lemmy.world 2 points 1 year ago

That can lead to another problem though, which is that if a developer knows a merge is only part of the whole change, it becomes easy to assume any issues will be handled elsewhere.

[-] learningduck@programming.dev 2 points 1 year ago

How do you improve on this?

1 bigger PRs, but with multiple smaller commits, so reviews can review by commits?

[-] midas@ymmel.nl 18 points 1 year ago

Really grinds my gears to get a review request for a huge PR with almost no context and no instructions on how to run the code / test data.

I've even had one guy ask for a review who hadn't even manually tested his own codes happy path. Sure he wrote some unit tests and those ran but once you actually tried using the code in the app all kinds of exceptions and weird situations came up. No idea how people dare do that shit.

[-] fiah@discuss.tchncs.de 8 points 1 year ago

No idea how people dare do that shit

perhaps we should introduce the PR Wall of Shame for exactly those situations. I mean, obviously you don't want to strangle productivity by naming and shaming people for every single small mistake, but such egregious violations like not even click testing the happy path should be used as an example of what fucking up looks like

[-] tatterdemalion@programming.dev 4 points 1 year ago

Playing the blame game, especially publicly, is a bad way to encourage psychological safety in a community. See Linus Torvalds for examples (he has recently become a bit softer with his feedback).

A better option is to make clear expectations of what a good PR looks like. Then if expectations are not met, you can give 1 on 1 feedback. Don't just blast a noob in public or you can leave emotional scars.

[-] fiah@discuss.tchncs.de 1 points 1 year ago

Don’t just blast a noob in public

true, but also when someone who's been around for a while and ought to know better, it can really help to remind them that they're not above the rules

[-] dreadedsemi@lemmy.world 17 points 1 year ago

The trick is to give it to another programmer.

[-] dudinax@programming.dev 15 points 1 year ago

True, but the 10 line change can me merged two weeks from now and it won't make any difference. If you let the 500 line merge languish for two days you'll have screwed up the work of three other programmers.

[-] bleistift2@feddit.de 14 points 1 year ago

I know this implies that the reviewer didn’t care to read the bigger PR, but I think this might actually be legit. If your PR is only 10 lines long, then chances are those lines are very dense, or intricate in some other way. However, if you submit 500 lines, then it’s probably mostly boilerplate code with trivial adaptions.

No-one in their right mind would submit 500 lines of substantial code.

[-] OwlPaste@lemmy.world 30 points 1 year ago

Oh sweet summer child...

[-] nul@programming.dev 6 points 1 year ago* (last edited 1 year ago)

You'd be surprised. Especially if the testing environment is not readily available or if automated tests are not functional and comprehensive, large code changes can be the norm. A developer may habitually hang onto their code until a big chunk is complete, at which point it will take heaps of debugging to uncover where the errors are. This is why we need IaC to quickly create testing environments that closely mirror prod, and trunk-based development to ensure code changes are small and issues are caught as early as possible.

[-] kabat@programming.dev 5 points 1 year ago

Has no one here ever worked on a new project or even a new feature in a decently sized codebase? Working exclusively in maintenance / minor change mode has to be exhausting.

[-] bleistift2@feddit.de 5 points 1 year ago* (last edited 1 year ago)

Depends on what you classify as “minor change”. When I took up my first professional project, I found a plethora of little things to improve which would make users happy. That was very satisfying.

On the other hand, writing yet another module that displays a list of Foos, lets the user create a Foo, show the details of Foo, update it, and delete a Foo, becomes dull quickly, despite being a “new feature”.

[-] BadAtNames@lemmy.fmhy.ml 1 points 1 year ago

I have worked on non-trivial (aka took 10-12 people over a year to even deliver an alpha) greenfield projects, where I literally made the first check-in into the repo.

The only 500+ line PRs I raised was auto generated boilerplate code, or renaming something.

I don't understand the optimism of devs who spend weeks writing code without bothering to test anything they've written. Unless you're writing utterly trivial BS, how does one have this level of confidence in their code? And if you did bother to stop and test, why on god's green earth would you not raise a PR? Why wait till you have thousands of lines of code before asking for feedback?

[-] Bagel@lemmy.ca 3 points 1 year ago

500 line of substantial code happens once in a while in my team. Goes beyond that sometimes.

[-] HorseWife@midwest.social 3 points 1 year ago

Saaaame, pretty frequently on my team actually

[-] Ser_Salty@feddit.de 8 points 1 year ago

If a programmer does 500 lines, how much work will they get done before their heart explodes?

[-] GnothiSeauton@lemmy.world 4 points 1 year ago

@bleistift2@feddit.de is right. What the lines are is very important. But it also matters over what time period.

If we do say, 20 lines per workday for a month, your heart will be fine!

Your nose might fall off though.

[-] bleistift2@feddit.de 3 points 1 year ago* (last edited 1 year ago)

Depends on what you’re doing. 500 lines in an Angular (a web frontend framework) application gets you a read-only view of a list of entities, maybe searching and filtering.

[-] veleon@programming.dev 5 points 1 year ago

FYI: OP is talking about a different kind of line than code

[-] bleistift2@feddit.de 3 points 1 year ago

I am stupid 🤦 Thanks for pointing it out.

[-] MaybeFrederick@lemmy.world 6 points 1 year ago

All jokes aside, you rather want those 500 remarks even if you only implement 2 of them. That’s the case for atomic commits

[-] Astiolo@lemmy.fmhy.ml 4 points 1 year ago* (last edited 1 year ago)

This can be called the bike shed effect or Law of Triviality. It's not just programming where a simple digestible idea will be contested because it's easy to poke holes, while something more complicated and more consequential is much harder, so it gets little to no resistance.

[-] BenLloydPearson@programming.dev 3 points 1 year ago

Y'all need to get yourselves some PR review automation in place. Stop wasting time on trivial reviews and requesting changes for common problems so that when you ping a colleague for a code review, they know it's important rather than a simple request for a thumbs up.

[-] Simulation6@sopuli.xyz 3 points 1 year ago

A couple weeks ago I was tasked to review my own code. It was over 10 lines, so I just passed it.

[-] Teeetris@feddit.nl 2 points 1 year ago

Lol 😆😅

[-] zos_kia@lemmy.fmhy.ml 2 points 1 year ago
load more comments
view more: next ›
this post was submitted on 02 Jul 2023
1079 points (98.6% liked)

Programmer Humor

18388 readers
1167 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