this post was submitted on 14 Nov 2023
730 points (97.0% liked)

Programmer Humor

19155 readers
1647 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
730
Merge then review (programming.dev)
submitted 10 months ago* (last edited 10 months ago) by agilob@programming.dev to c/programmer_humor@programming.dev
 

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

you are viewing a single comment's thread
view the rest of the comments
[–] silencioso@lemmy.world 20 points 10 months ago (3 children)

Before everyone loses their minds, in Extreme Programming there are safeguards other than PR reviews. Before you submit a PR, you are supposed to have written the tests and to have written your code with pair programming, so your code already has some safety measures in place. On top of that, when you merge and deploy, more tests are run, and only if all of them are green do your changes go into production.

[–] redcalcium@lemmy.institute 14 points 10 months ago (1 children)

Pair programming? Then the code is already reviewed.

[–] hglman@lemmy.ml 1 points 10 months ago

Yes, that's part of the point. Dumping all at once into a merge and asking people to comprehend it all isn't particularly realistic.

[–] agilob@programming.dev 5 points 10 months ago (3 children)

you are supposed to have written the tests and to have written your code with pair programming,

I commented out the tests because they were failing, pipelines were green so I merged. Now it's running on prod. What do you do?

[–] silencioso@lemmy.world 16 points 10 months ago

Fire you for destroying the tests. It's intentional sabotage.

[–] pomodoro_longbreak@sh.itjust.works 6 points 10 months ago

Give you public kudos for moving fast and breaking things. We need more fearless cowboys like you around here

[–] Blackthorn@programming.dev 1 points 10 months ago

I would fire you for incompetence and sabotage. Problem solved.

[–] MeanEYE@lemmy.world 3 points 10 months ago (1 children)

You lost me at "pair programming". Having tests for what you can test is fine. But there's code that simply can't be tested, or at least not easily at which point you are just wasting time. Open source mantra is always great in my opinion... release early, release often. In addition to that have a test version of your software before you push it to production if there's sensitive data. That's usually good enough to catch issues.

And he's right, reviewing changes before merge just takes time and resources away from project while the master branch keeps moving. Merge, if there are issues, whoever submitted the change is obliged to fix it. You can always checkout earlier version.

[–] dbilitated@aussie.zone 2 points 10 months ago (1 children)

I just made a github action that merges anything updated in master into feature branches automatically. you get pinged if there's a conflict but the automerge keeps drift to a minimum so it's less common and fixed sooner.

better than merging poorly tested/reviewed code.

and yeah, a small team of superstars doesn't need reviews so much, but most teams have a range of devs with different levels of experience and time working with particular parts of a large codebase. Someone more senior or more expert derisks people picking up tickets and improves code quality.

it also leads to plenty of good conversations about the best way to implement, so overall it's a win.

[–] MeanEYE@lemmy.world 2 points 10 months ago

Well, Git was designed to branch out, not be a single repo with bunch of users. So one team can have a local repo, that in turn gets merged into big one, etc. Structure matters as you say. Small experienced teams move fast. Big teams require a lot of management and supervision. I still think it's better to split people up into small teams and give individual tasks, or let them pick tasks that need to be done.