Reverse Pull-Requests | Hacker News Reader

Reverse Pull-Requests

ideas.riverglide.com

8 pointsmax-m24 comments

This is the worst process I've probably ever seen. Only one person (or pair) can work on the code (in main) at a time for this to work. It's more effort than just making a branch and merging it to main, and it buys literally nothing.

What is wrong with just creating feature branches and PRing them to main? It's literally the default workflow.

Then when you merge to main you run the CI/CD just like this, but without having to go back and create a "snapshot branch" of main before you started. Also, as you scale you almost certainly have commits from multiple people in main by the time you do this breaking the whole "reverse PR" process.

You're misreading it, you review in batch and in retrospect after it's been put into production (with associated incremental/canary rollout, feature flag etc safety controls) and trunk progresses freely

Good luck passing a SOC 2 audit with that kind of workflow!

What happened to curiosity for finding secure ways to push workflows forward? People said the same things before continuous deployment caught on and now that's common enough and definitely SOC 2 capable.

I believe it's doable with production rollout controls that limit customer exposure to the code changes and the code's access to data (possible these days with things like pg row level security controls, and blue/green routing etc. - already valuable practices), and ensuring the batch code reviews happen before customer rollout. Even with these controls and incremental rollout, there is tremendous benefit to getting the code deployed into production environments and enabling this review workflow. The point then is to satisfy remaining security controls while in production, under these type of controls.

Because the code is written in pairs, there’s two pairs of eyes on all code. I’m not familiar with SOC 2 specifically, but that’s sufficient for other security controls I’ve seen.

The purpose of this review is to create consistency across their large number of services. It’s really just a convoluted way of using GitHub’s PR system rather than doing a diff and talking it over.

Personally, I think it’s overly complicated, but I think the root cause of that complexity is having over 40 microservices owned by a single team, each in its own repo. It’s resulted in a lot of boilerplate, aka duplication, that they might be able to factor out if they used a monorepo. (Or a modular monolith.)

Why do people try to come up with these byzantine git workflows?

> However, what was missing was wider visibility of changes being applied to the code.

That is what is said to have been missing. The solution was not to use tags or feature branches, gitk or other visualization tools that could easily show changes being applied to code from any point to any other point. It was to make up this "reverse pull request" Rube Goldberg scheme.

This idea is ahead of it's time.

By which of course I mean that I expected it to take a few more years before a new generation forgot where we all came from to arrive at git, and reinvents cvs for no other reason than to work the opposite of git, because by then git will be old and whatever is the opposite of something old is new and "progress".

Just reading this article made me nervous.

More microservices than programming pairs.

Deploying straight to production.

Maybe I just like the old school simplicity, but this seems like a terribly convoluted process, to justify some sense of development momentum that people seem to be addicted to.

I’m not entirely sure if I got this right, but this system sounds a bit … weird. Everything is “tested” in production and then, at some arbitrary time, another team does a code review of the code that’s already in production and possibly broken? It just seems completely backwards.

Sounds like real innovation toward actual continuous integration and continuous deployment! Indisputably better for the typical DORA DevOps metrics, the trick being relying on other quality controls and not treating code review as a quality control for regressions outside of feature flags.

We operated this way back in the day building Canv.as and DrawQuest but without the snapshotting, that's a nice development

Wish more teams would explore this type of development. Everyone is stuck in the git kernel dev model practices. It probably sounds weird because it's unusual, not because it's a DevOps bad practice (it's quite on the leading edge).

It also won't pass a SOC 2 audit if people can just commit anything and ship it to production with no oversight.

https://news.ycombinator.com/item?id=32697298

It sounds like they’re using XP, or something similar. In XP, the code is reviewed and tested as it’s written, using pairing or mobbing for the review and TDD for the testing. (XP also uses frequent pair switching and collective ownership to get more eyes on the code, but it’s not clear to me whether they are or not.) So when the code is deployed, it’s already been reviewed and tested.

The review they’re talking about here comes later, as a way of keeping the team in sync. It’s not the main code review mechanism.

Do people really want to work in pairs all the time? I feel like it would get tiring if not just an occasional thing. There's also value in being able to be an "individual contributor"

I'm OK with pairing a significant amount of time if it's async. I find traditional sync pairing draining after an hour or so. If you are pairing all the time something has to be off, it's not efficient at all.

How exactly is it still "pairing" if you are doing it asynchronously?

My understanding and experience with "pairing" is that it pretty much has to be synchronous because generally at least two people should be looking at the same screen while generally one is providing inputs (mouse, keyboard) and the other is providing high level guidance and looking out for low level mistakes.

Doing it asynchronously seemingly would not dispense with sharing the screen and one person handling inputs which seems like it would be even less efficient.

I imagine they mean when you work with some high bandwidth channel with another developer, and work independently on the same thing.

One type of async pairing I’ve done is “one person writes tests and the other writes the code to pass said tests - then swap” Im sure there are others but this is all I’m familiar with - it works ok and has the benefit of being sorta TDD, or at the least making sure tests are written

this branch strategy works pairing or not

This has one glaring problem (only works for one feature). The article mentions it, says “wasn’t a big problem for us” and that’s it.

The real solution might be to create an after the fact feature branch and cherry pick all the relevant changes. That has some risk of breaking under high churn, but the feature branch doesn’t even need to build or pass tests, only convey a design, I suppose.

But in the end: why not just use feature branches? As soon as the number of developers become big enough to worry about passing knowledge around, waiting 12 or 24h for changes to land in master is a tiny nuisance compared to broken master builds or reverse PRs.

You could merge PRs branches (not PRs itself) into a shadow-master branch. Every push to any PR creates a fresh shadow-master with selected (labeled) PRs's branches merged. We call this an acceptance branch. Each acceptance branch could have its coresponding staging server.

You could have several acceptance branches with different PRs merged. Each PR can be included/excluded (labeled/unlabeled) back and forth from the acceptance branch (get a fresh one on every label attach/detach)

That's what companies I worked with do. And I even built a SaaS app for that, though they don't want to publicly advertise the app.

Similarly gitlab supports merge results pipelines. https://docs.gitlab.com/ee/ci/pipelines/merged_results_pipel...

Yes, partly!

> Merged results pipelines can’t run when the target branch has changes that conflict with the changes in the source branch.

Yeah, the app I built even use git's `rr-cache/` (rerere feature) and have gui for resolving conflict and marking it as resolutions. Pretty neat.

This looks like a try in recreating audit workflow (in a bit overcomplicated way).

https://secure.phabricator.com/book/phabricator/article/revi...

Yeah, I do miss Phabricator :(

Reply on news.ycombinator.com