Comment by derefr | Hacker News Reader

Comment by derefr | Hacker News Reader

Parent Full thread

> treating PRs as communication

I kind of wish that there was a type of thing like a PR, but marked in a way where you couldn't actually merge it. It'd still get built by CI, if such a thing was configured; but the point of the submission of such a "proposal with prototype" object would be to discuss whether the design represented by the prototype implementation is a design worth going with.

The actions applicable to such objects would be "accept and close" or "reject and close." You'd be able to have several of these objects that live under a given issue (i.e. several potential solution-designs to the same problem), and—as long as the issue has at least one proposal-with-prototype object under it—the issue would be in a "pending" state until one such proposal object was Approved, and Approving one proposal object would Reject the others.

The point of this would be to replicate the thing that people go through with design discussions on mailing lists when they send in code samples to explain their designs—but with those code samples being working, buildable code, such that the properties of the design proposal can be tested against the current implementation and against any alternative proposals.

You could call these objects "RFCs" :)

My previous team had a convention of putting a prefix "[DO NOT MERGE]" on the subject line of RFC-style PRs. Worked well enough, though it would have been nice to have the tool enforce that also (even a checkbox like "prohibit merging" would have worked).

Gitlab actually enforces this for PRs with [WIP] in the title

Thank you for that tidbit of knowledge. Learned something tonight.

There's a browser extension that does something similar for github too. At least for Chrome, anyways. I imagine it works for other browsers though.

Absolutely, this. We did the same thing -- [DONT MERGE] or [WIP] in the title.

That way you could get feedback from other devs, and pushing commits to your PR would trigger CI runs so you could be made aware of any build failures you were causing. (The builds took hours, sometimes, so running the full test suite locally was impractical)

We used Github but I'm sure it would work in a lot of workflows.

This is pretty conducive to a Gerrit (https://www.gerritcodereview.com) workflow. If unfamiliar, Gerrit essentially treats each commit as it's own PR or "change" in Gerrit terms. It uses git's refspec as staging area so each commit basically becomes a mini branch. Changing a "change" then requires amending the commit and pushing a new "patch set".

So what my team does when we want to prototype something is we create a change and then push separate patch sets for different solutions (there is a nice diff tool to view differences between patch sets). The CI runs for each patch set and we then discuss the merits of each approach and "abandon" (reject) the change and use it for reference going forward.

We sometimes have this use case at work where we want to discuss a potential idea and get feedback without merging anything. What we do is open a PR and decline it directly (I'm using bitbucket lingo). This way it can't be merged, and the discussion diverges from the particulars of the diff and focuses more on the idea and the architecture of the change

I was just about to link to something like this[0] before I read your last sentence.

[0]: https://github.com/rust-lang/rfcs/pull/911

Hmm. I suppose your CI and code review tools allow code on branches different than master?

So you can directly use them to show a what-if PR. I did; it worked, that is, sparked a discussion and led us to designing things in a better way.

Yeah, I was picturing GitLab's CI/CD workflow here, where the RFC objects would get Review Apps built from them.

My objection to using PRs for this is that people think the point of a PR is to merge the code, and people tend to nitpick the code during code-review with the goal of making it clean enough to merge.

The idea of a separate RFC object is that, unlike a PR, you literally cannot merge an RFC, so there's no temptation to nitpick, or really to talk about anything other than the design. It much more closely mimics the social mores of a mailing-list thread discussing a code snippet.

Also, being able to explicitly track Approved and Rejected RFCs on a system level would be nice, to know what discussion needs to be referenced when doing the final implementation. If you just used "what-if" PRs, both the chosen and not-chosen designs' PRs would just end up in the Closed state, and would show up equally in search. Ideally, Rejected RFCs would be filtered out of search by default.

Interesting idea. We already sort of have a special-case MR (WIP) where merge is disabled until the work-in-progress status is removed. It could be interesting to have an RFC equivalent where no merge is clearly ever intended, but you still get the conversation flow, review app, and so on.

Definitely might be useful for teams/projects organized enough to have an RFC process.

Reply on news.ycombinator.com