[deleted]
Why curl closes PRs on GitHub
21–30 of 41 posts
Re: Why curl closes PRs on GitHub
#22[deleted]
Re: Why curl closes PRs on GitHub
#23I'm glad to see someone else annoyed that GitHub doesn't allow commenting on the commit messages directly. One of the things I miss from Gerrit.
Re: Why curl closes PRs on GitHub
#24Earlier quoted context omitted.
We also do squash and merge by default in my company. Each PR is supposed to represent one and only one feature and/or change. Can you please better explain the disadvantages of this? Of which editing are you talking about? What needs to be edited after the squash and merge?
Bisecting is more efficient when individual commits are preserved, IMO/E.
Inevitably you will encounter (a) a junior developer who considers commits to be 'save states' rather than individual logical changes or (b) someone who is used to squash merge and did not think commit history mattered.
Without squash merge, it then becomes very likely most commits do not even build making bisecting a bit of a nightmare.
Re: Why curl closes PRs on GitHub
#25I was working at a company that hired a lot of people new to dev. So one guy, comes to us saying his commits arent matching the standard they want. I see his commits, and I see like 50 of them that just say stuff like "fixing the bug", "finally fixing the bug", I tried to tell him, try to describe what the code is changing / doing with this commit as if you had to explain it to you mom. He gave me a look like I was crazy and proceeded to ask someone else for help. I assume he's still committing with awful messaging that matches their desired format.
Re: Why curl closes PRs on GitHub
#26Couldn't they just edit the PR branch directly? When I make PRs I have an option "Allow edits by maintainers" which I assume would give them write access to my branch? Edit: "We COULD but we won’t" actually addresses that, but it just wasn't obvious to me that they were referring to that feature. I can't delete this comment anymore.
They explicitly call out not wanting to do this in the blog post.
Re: Why curl closes PRs on GitHub
#27Interesting. I made some contributions to the Zephyr RTOS project early this year, and they had a different approach to the same problems. It seems like Curl makes it the maintainers' job to do the rebasing and commit-style corrections. Zephyr, on the other hand, put it on the pull request's creator. I ended up force-pushing and rebasing half a dozen times because I had to learn all their rules the hard way. I don't…
> Zephyr's approach lets the maintainers focus on more important things than commit-style yak shaving, while curl's approach lowers the barrier to entry for inexperienced contributors.
Depends on the person I guess but I often find it less exhausting to just fix up minor issues myself than to get a driveby contributor to adhere to the project style.
Re: Why curl closes PRs on GitHub
#28Interesting. I made some contributions to the Zephyr RTOS project early this year, and they had a different approach to the same problems. It seems like Curl makes it the maintainers' job to do the rebasing and commit-style corrections. Zephyr, on the other hand, put it on the pull request's creator. I ended up force-pushing and rebasing half a dozen times because I had to learn all their rules the hard way. I don't…
There are some code systems that force a rebase-centric workflow like Gerrit, where you literally can't push more changes, unless you want to make separate review requests that get queued (which can be desirable) Coming from those better systems, the review interface of Github feels like a badly implemented afterthought.
> Coming from those better systems, the review interface of Github feels like a badly implemented afterthought.
This really goes for most of GitHub's additions. Issues, releases, wiki all feel like the bare minimum needed to claim it has those features.
Re: Why curl closes PRs on GitHub
#29Earlier quoted context omitted.
Bisecting is more efficient when individual commits are preserved, IMO/E.
Inevitably you will encounter (a) a junior developer who considers commits to be 'save states' rather than individual logical changes or (b) someone who is used to squash merge and did not think commit history mattered. Without squash merge, it then becomes very likely most commits do not even build making bisecting a bit of a nightmare.
That is where you teach that developer to rebase his commits into logical and self-contained units before merging the PR.
Re: Why curl closes PRs on GitHub
#30Couldn't they just edit the PR branch directly? When I make PRs I have an option "Allow edits by maintainers" which I assume would give them write access to my branch? Edit: "We COULD but we won’t" actually addresses that, but it just wasn't obvious to me that they were referring to that feature. I can't delete this comment anymore.
They explicitly call out not wanting to do this in the blog post.
I didn't notice that they were referring to that feature, it's too late to delete my comment now...