this post was submitted on 14 Nov 2023
646 points (96.9% liked)

Programmer Humor

19618 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
 

Move fast and break things.
Merge vulnerabilities.
Double the work.
Merge code without tests.
Anything, but don't let code become stale.

(page 3) 31 comments
sorted by: hot top controversial new old
[–] Deifyed@lemmy.ml 2 points 1 year ago (9 children)

I kind of with the sentiment. Review pre merge though, but only block the merge if there are serious faults. Otherwise, merge the code and have the author address issues after the merge. Get the value to production

[–] jjjalljs@ttrpg.network 2 points 1 year ago (1 children)

This is some poe's law shit. I can't tell if you're serious or just committing to the bit.

[–] Deifyed@lemmy.ml 1 points 1 year ago (1 children)

Sorry about the confusion. It's not sarcasm. I'm just sick and tired of people blocking my PR because of an argument about wether the function should be called X or Y or Z or D

[–] jjjalljs@ttrpg.network 1 points 1 year ago

Ah. Yeah those kind of nitpicks are annoying. We try to specify when comments are blocking or non blocking on reviews.

But I definitely block a lot of reviews over no tests, bad tests, no error handling, failed linting. And the occasional "this doesn't do what the ticket asked for"

[–] zalgotext@sh.itjust.works 1 points 1 year ago (2 children)

Get the value to production

Ugh, not this SAFe Agile (tm) cultist bullshit. The "value" is working, bug free code, which you get when you put it through review and QA before it gets to production.

[–] Deifyed@lemmy.ml 0 points 1 year ago (1 children)

There's often features and bug fixes worth more than the ones introduced in the PR. I've yet to see bug free code just because it's went through review and QA.

[–] zalgotext@sh.itjust.works 1 points 1 year ago

Surely you've seen bugs caught because code went through review and QA though. Those are bugs that would go into production if following the "advice" in this post.

load more comments (1 replies)
[–] kautau@lemmy.world 1 points 1 year ago

This only works if the merge is being done to staging builds that are continuously tested by a QA team before they go to production, with carefully planned production milestone releases. I work for an emergency management SaaS company. If we just merged all lightly reviewed code into production without thorough QA testing, there’s the possibility that our software would fail in production. This could cause aircraft in major airports to crash into each other on the runway, or a university to respond poorly to a live shooter situation, or the deletion of customer data about COVID vaccine efforts, etc

load more comments (6 replies)
[–] petrescatraian@libranet.de 2 points 1 year ago

@agilob code is like wine. You let it out in the cold and it gets better over time by itself.

[–] null@slrpnk.net 2 points 1 year ago

The subtle Linux-flex in the screenshot.

[–] Blackthorn@programming.dev 2 points 1 year ago (4 children)

Probably unpopular opinion, but peer reviews are overrated. If coders are good AND know the project, the only thing you can do in a PR is nitpicking. They are more useful for open source collaborators because you want to double-check their code fits with the current architecture. But people here are reacting as if peer reviews could actually spot bugs that tests can't catch. That happens rarely unless the contributor is junion/not good.

[–] the_artic_one@programming.dev 1 points 1 year ago (1 children)

If coders are good AND know the project

Those are some pretty big ifs.

[–] Blackthorn@programming.dev 1 points 1 year ago

Code review can't fix incompence though. I lost count of how many times my boss told me "review that PR well because X is not very good". Also my point is that they are overrated, not that they are useless.

load more comments (3 replies)
[–] Skates@feddit.nl 1 points 1 year ago* (last edited 1 year ago)

That's nice but it goes against our quality standards and the international quality standards we are charging the client extra for adhering to, the line you're trying to merge into is stability and needs CCB approval for the merge, and the client has specifically requested only showstopper-level bugs be addressed for stability lines. You know what, I have neither the time nor the crayons to properly explain this to you, a consultant that supposedly knows the business. Pack your shit, you're gonna have a wonderful time posting this crap on LinkedIn instead. #gitshiton

2 days before, at Pete Hurrd former job

It can work if you have a test zone and only a small amount of people work on a given code base.

Also checks to ensure the code compiles and tests pass before merging, as some quality gateway.

load more comments
view more: ‹ prev next ›