Hacker News new | ask | show | jobs
by davexunit 4421 days ago
One of my biggest problems with pull requests is the tendency to have to merge a bunch of "Fix typo", "Refactor foo" style commits.

I prefer the workflow for sending patches on mailing lists: Email patch, get feedback, make new commits to address feedback, rebase (important step), repeat.
2 comments
jimktrains2 4421 days ago
Why is that inherently better than a PR? I've had people give me feedback on my PR, I make the changes, then update the PR with the correct SHA.

I'm honestly curious. PR are a tool, you can misuse any tool.

link
davexunit 4421 days ago
You can force push to your remote branch, but I personally like to keep my branches local to my machine and simply send along .patch files that can be applied with git am.
link
Mithaldu 4421 days ago
> I personally like to keep my branches local to my machine

Are you saying you prefer to deny others the ability to directly interact with the same git structure you have in their git tools (by simply fetching from a remote), instead forcing them to apply a patch from an email and hope everything goes right? If so, why would you do that? This seems like it's a little idiosyncrasy you have that has little to do with the actual topic at hand.

link
davexunit 4421 days ago
No, I'm not saying that, but I contribute to some projects that do not use GitHub. I send patches to a mailing list, the maintainers review them, and then apply them when they're acceptable. I'm not saying that this is the only way to do it, but I'm pointing out that this workflow has a nice side-effect: clean patches. With GitHub's pull requests, you would typically respond to feedback by pushing some new commits ("fix typo", "add test", etc.). When I am sending patch files to a mailing list, I respond to feedback by making new commits locally, squashing them into the original patch set, and generating the patch set again. This keeps the git history clean and focused. The patches stand alone. Tools like Gerrit can also do this. I'm not endorsing one true workflow, but I am saying pull requests are inadequate.
link
Mithaldu 4421 days ago
> With GitHub's pull requests, you would typically respond to feedback by pushing some new commits ("fix typo", "add test", etc.).

Sounds like your problem isn't with pull requests, but with neophytes you have experienced in github projects you looked at, who do not rebase and force push.

In the majority of projects (excepting those run by neophytes) i participate in, whether on github or off, there has always been a clear line of pull requests required to be clean, meaning not only should the commits not contain cruft-cleanup, they also have to be on HEAD, contain Changes-file adjustments, contain tests, etc. When patch makers get these instructions and force-push that kind of thing, it's a breeze for maintainers to do a fetch, then compare their local marker on the requesters branch with the new state of the remote branch.

Having a requester go "no, i only send emails" in that situation would not be constructive.

link
entropy_ 4421 days ago
You don't need to force push at all. You could just apply the changes to your PR in a different branch(completely local, rewrite history as much as you want, etc, etc) and then submit a second PR with that.
link
davexunit 4421 days ago
A whole new PR? That just shows how broken PRs are if that's the solution to this problem.
link
jimktrains2 4421 days ago
It's _A_ solution, but not a good one. I think most people consider it OK to force-push a PR branch and update the existing pull request.
link
davexunit 4420 days ago
There's a lot of opportunity for destruction with a force push, so it seems weird to consider it acceptable in a code review workflow.
link
joshdance 4421 days ago
To add another perspective, I very much dislike getting emailed patches. Creates more work for me. We use Phab add getting an email with a link, clicking the link and immediately seeing the diff is super easy. Reduces friction to increase use.
link