727
Merge then review
(programming.dev)
Move fast and break things.
Merge vulnerabilities.
Double the work.
Merge code without tests.
Anything, but don't let code become stale.
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
Hahahahahahaha. Sorry, you've merged, next ticket, PM needs shiny results for execs this QBR!
This is how bug backlogs grow.
This exactly. By the time they notice a problem you are three tickets down and on to the next sprint.
Yeah, I see your point. Maybe my employers are different, it's never been an issue explaining why the ticket isn't closed just because the PR is merged
Oof, I felt this
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
This is some poe's law shit. I can't tell if you're serious or just committing to the bit.
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
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"
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.
There is no value in spaghetti piled on top of rotten spaghetti. Tech iCal debt is real and if you're just shippin it and plan to fix it later, y'all gonna have a bad time. Nothing more permanent than a temporary workaround.
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.
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.
I'm saying identify the bugs through review, and fix them. Just do it in a new PR unless they are critical