Disabling direct pushes to master

I think we should disallow pushing directly to the master branch and instead require all changes go through a PR. I find this to introduce minimal overhead (it’s just two clicks in GitHub to create and then merge a change) and be very useful to prevent mistakes (i.e. accidently pushing to master when I intended to push to a branch). It also encourages devs to let their changes run through the PR validation checks though doesn’t require it.

Also, I get emailed for every PR created which I find useful for keeping up on what’s going on. The same isn’t true for direct pushes to master.

I know that personally I wouldn’t want this for the LLVM repo: the ability to revert patches and landing build fixes and other small things quickly is valuable. I’d be careful about setting up workflows that wouldn’t survive an integration in the monorepo (isn’t it a goal of being an incubator project?)

I’ve not found the pull request process to hinder my agility if I don’t wait for feedback or the PR “gates” to run. Both are optional and if I’m choosing not to wait I usually add a comment in my PR with the justification for not waiting. This adds a big enough hurdle to help prevent mistakes (and maybe wait for the rather fast PR checks to complete); but, not so large as to impede rapid fixes and reverts.

It wouldn’t work well for LLVM proper since going through Phabricator is a significant enough hurdle so as to hinder agility. GitHub PRs really introduce at least an order of magnitude less friction.

We’re already using PRs instead of going through Phabricator which wouldn’t survive integration into the monorepo (currently). Also, there’s something to be said for incubator projects doing things differently as experiments. And hopefully by the time CIRCT may be mainlined, LLVM (or at least part of it) will be off of Phabricator. (Though probably not.)

I read your opinion, but I have a different experience though: Phab is zero friction to me, much less than having to fork the repo and working with two remotes (you could push branch to the main repo, but that’s not great and I wouldn’t want this).

Working with two remotes is certainly a PITA but I end up doing the same for LLVM mainline development. I push my changes to my personal fork, stage there, then use arc to push my changes on that branch. (PRs would be more convenient for me.) I do this to get backups, access my changes from anywhere, and (frankly) use the diff viewer in GitHub since I like it more than anything I run locally.

I actually have been considering pushing to branches in the main repo which (IMO) isn’t too bad in practice – I don’t mind people being able to easily see my work-in-progress.