I believe at least half of people are stuck there out of sheer laziness. There are other options. We just don’t want to deal with them.
Personally I’m not big on GitHub, and I avoid using it. Only if there’s a project that does, and I need to work with them. And I believe 90% of my usual (direct) interactions lately isn’t there at all.
Now, I know I’m probably an exception. And I don’t know why not many others would want to reduce g GitHub use as well. But they don’t and here we are.
Not because it provides any superior experience - I simply don’t have any big-enough projects that would require something long-term stable. If I did, I would probably pick something like Codeberg or GitLab though, because in the past they never seemed to be so focused on doing everything, and they didn’t push “social” features that much and focused more on developers using the platform.
Now that I think of it, a lot of my reasons to avoid GitHub are just “old man yelling at cloud” type.
But I stand by the claim from my previous comment - most of us are just lazy to use something else. A lot of repositories don’t use any of the extras done by GitHub, so there’s no reason to use something else - except for “well everybody else is there, so we have to”. It takes like 3 minutes to clone a repo somewhere else then GitHub, and do your work there. But most people don’t even bother. And so GitHub remains almost a monopolist in this market. Which we all know how it ends.
You’d need capital to make something relatively smooth to use but now that the option space has been mostly explored anyway, no reason you couldn’t run the same track with a 10-20 person team.
Still you’d need a differentiator. I’m not sure whether novel technology (different vcs, nix/nx/turbo) or maybe novel process (native support for monorepo/trunk/patch development) would be it.
It won’t be easy to prevent 3rd parties from scraping github or other code sites to collection repositories for AI model training. In the long run, if you are publishing your code, it will end up training AI models.
I live in a jurisdiction where there is legislation on copyright for ML and data mining. The EU TDM copyright exception says that while ML and data mining for academic research purposes can ignore copyright, commercial data mining and ML training has to respect a “machine-readable opt-out”. There’s a W3C proposal for a protocol that serves such a machine-readable opt-out to scrapers, and several other proposals for the technical side as well. I’d gladly host my code somewhere that had some competent people dealing with all that (perhaps with IP blocks for poorly-behaved bots).
Of course, more cynically: I fully expect the AI companies will ignore those opt-outs and steal everything anyway.
Another thing I’m missing in github PRs is iterations. I’m of the school of thought that git commits should be well-crafted tools of communication instead of periodic check-ins: basically, within a PR, diffs should be decoupled from the actual commits that will land in the target branch. So when people comment on my PRs I usually go back and modify past commits in-place with rebase instead of adding new commits responding to their feedback. But then when I force-push these changes to the PR, people can’t see the actual changes I made in response to their feedback.
Perhaps SourceHut is right and the answer is really to RETVRN to git-send-mail. Federated stuff is really having its moment these days, and what’s more federated than email?
But then when I force-push these changes to the PR, people can’t see the actual changes I made in response to their feedback.
They can, and over the years github has made it very slightly more visible (it used to be really well hidden), although it’s quite fragile: if you go on a PR where you forced-pushed stuff, in the history you’ll see something along the lines of
The (now underlined) “force-pushed” is a link to the diff for the force push, that is it diffs the HEAD of the PR before and after the force push. Now the big trap is if you rebase on top of a branch which has had updates then those updates will be part of the diff, but if you only rebase “in place” for the purpose of edition, it’ll work fine.
Kinda.
One bit that’s dearly missing is comments may or may not live across the change, and so relating the comments to the changes can be difficult.
Oh man, I would hate that. I like to see a new commit on the PR because then I can look at just that diff to see if / how my comments were addressed and what the code looks like now instead of having to re-review the whole thing or sort through a larger diff for the small part I want to see.
I think what the the comment you’re responding to is describing is a system where you get these kinds of diffs, even if you use other workflows than adding a new commit per round of review.
You can use fixups, ‘git-autofixup’ would even split a commit and fixup the correct commit in the history most of the time. Then when reviewers are happy you can rebase and squash the fixups only.
You are right, this functionality is all built into git already…
On a system like Gerrit or Phabricator, each PR has a branch; previous revisions of a PR tell you which commit that branch pointed at sometime in the past; and there’s no requirement or expectation that earlier revisions are parents of later revisions.
This functionality exists in git via the reflog. And remembering that git diff can show the differences between any two arbitrary commits, you can use
$ git diff $BRANCH_NAME@{4} $BRANCH_NAME
to see the changes between what your branch pointed at 4 revisions ago and what it points at now — even if there is no commit history in common between the two, because of rebasing, amended commits, etc.
Doing such a commit temporarily as a hack can be ok I guess. But there’s nothing worse than doing a bisect only to find that the bug was introduced in “fix pr feedback”
One thing I’ve found is that people who are very invested in using GitHub (or any other forge) can often conflate the unit of review and the unit of change.
For someone who isn’t familiar, comfortable, or interested in using git as a local tool in its own right, and only use it as a way to interact with GitHub PRs, it’s very easy to lose sight of the git commit as unit of change, and start to think of the PR as the unit of change instead. And that leads to all of the suggestions like “never force push”, “make lots of small commits to record your progress”, “don’t worry about using something bland like ‘fixed some stuff’ as your commit message”. I personally much prefer the workflow (and resulting history) that @ahelwer is suggesting, but I’ve at least come to terms with “not giving two shits” about the commit log being a self-consistent world view, even if it’s not one I agree with.
In combination with Gitlens to see previous line commits, I now make lots of commits with messages that look like “fixup: Prev Commit Title” and I push that.
Once all feedback is addressed I make the rebase
You should use fixup! as your prefix, it’s the one generated by git commit --fixup and git rebase understands that syntax: if you invoke git rebase --autosquash it’ll automatically move the fixup commits after their subject.
Also squash! (if you want to edit the message of the merged commit), and amend! (if you want to completely replace the original commit message).
Annoyingly git-send-email doesn’t actually transfer commits. It allows creating a similar commit with the same diff.
I would love it if you could actually send a commit by email, such that as long as you have the base commit you could reconstruct the same commit when applying. This way merges and other git operations work more cleanly as Git is aware that they are the same commit. This also makes it clear what the intended base is, sometimes it can be annoying to find where a patch applies cleanly.
I would love it if you could actually send a commit by email, such that as long as you have the base commit you could reconstruct the same commit when applying.
I think git-send-email does that if you use the --base option.
Thanks for sharing that flag, I’m going to use that from now on, listing the base is a nice bit of metadata for anyone trying to use the patch. In fact I am going to try setting format.useAutoBase=true and see if it breaks any automations that I use.
However it doesn’t seem to perfectly construct the original commit.
But the resulting commit is different. It seems to just be the “committer” info that is different. Running a diff I see that the committer tag is different (current time, and probably updated to whoever I am) and the gpgsig is freshly generated. Both of these bits of information were never part of the original patch so it is clear that there isn’t enough information to perfectly construct the original commit.
What I would like is to perfectly reconstruct the original commit, then I can create a merge commit or similar if I want to record who committed the patch to whatever branch it is becoming part of. Leave the original unchanged, but use a merge or similar as a record of how the original work was integrated into the project.
Ah yes, I forgot about the committer metadata. There’s the commit time as well as their name and email address. git am has --committer-date-is-author-date but no option like --committer-is-author
In practice ActivityPub & XMPP come to mind. Almost all users of email are using one of like 4 services where as these two protocols seem to have a lot more self-hosting & small instances relative to the large ones. Email sees more general usage so the spam prevention mechanism are high further incentivizing just moving to a provider that works with the Google & others.
Now extending to a real-time, collaborative, federated, MUC-per-patchset in a hypothetical git send-xmpp is something I would love to see. :)
Bitbucket has handled this nicely for years, at least the version descended from stash. But evidently that’s not enough of a differentiator to encourage people to use it.
Basically I meant following the linux kernel patch review style, which probably means everybody uses mutt and is familiar with the concept of topposting and why it’s bad.
We could in theory fix and cleanup the commits locally and manually exactly the way we do now and then force-push them to the remote branch and then use the merge button on the GitHub site and then they would appear as “merged”.
I just do this. It makes contributors feel good, after putting in all that work to make a patch.
We could in theory fix and cleanup the commits locally and manually exactly the way we do now and then force-push them to the remote branch and then use the merge button on the GitHub site and then they would appear as “merged”.
GitHub could offer a Merged keyword in the exact same style as Fixed and Closes, that just tells the service that we took care of this PR and merged it as this commit.
Indeed, that is a thing Github can do internally (as you can “rebase and merge” or whatever via their green button and the PR will be marked as merged).
On a somewhat related note, I despair at the amount of people who does not bother to try to stick with how contributions in a project are done.
I started using Gitmoji to annotate my commits (IMHO it makes the git log far more understandable, and it’s less awkward to mix scopes compared to “conventional commit”). One would believe that when you send a contribution to a repository following a certain format for their commit messages, you would stick to that format as well. But in reality, it never happens, and on every PR I either need to force-push after editing the commit messages myself, or I need to ask the contributor.
Even when I put a CONTRIBUTING.md file in my repository, the rules are not followed. On hobby projects I try to not get too annoyed by that, but at work, when I have to waste a hour 3 times a week explaining to a coworker that no, I won’t merge the PR until they follow the rules they did not bother to even look for, it gets on my nerves.
NB: The Gitmoji is just an example, I have the same problem with the git rebase requirement instead of merging the main branch into your branch, or even the “you should update the test suite and documentation”.
For short lived PRs ‘git rebase’ is fine, but for long lived feature branches you’ll make more mistakes with rebase than with merge. It is up to the contributor what best suits their workflow (and it doesn’t help if maintainers give conflicting advice on ‘git rebase’ vs ‘git merge’)
If you fix a conflict the wrong way you will have lost from your history the “correct” commit (unless you can dig it up from ‘git reflog’). With a merge it is all still there and if you made a mistake fixing a conflict then you can see how the correct commit looked like and fix it up.
Plus with a ‘git rebase’ you’ll have to fix up the same conflicts over and over again, whereas with a merge you won’t have to (unless the merge introduced new conflicts).
If the commit history looks like a mess at the end of this then you can do “one” rebase at the very end before merging the feature brach to the main branch, with proper review and testing.
The problem is that solving conflicts is not shown in Github’s review UI (with either a rebase or a merge), and it is very easy to make mistakes there (or if someone really wants to, then hide something nefarious there)
As much as I don’t like patchqueues, it does solve the problem of commit evolution. You can always see the full history of a patch, and how it evolved over time, even if a mistake got introduced when rebasing the patch months ago, and you can see and comment on commit messages too. Unfortunately reviewing changes is very difficult, there is no good UI, and reviewing what is essentially a diff of a diff is very error prone, unless someone supplies a git tree with the patches applied for review too.
Pijul has some interesting ideas to solve these problems, but I’m not sure whether it is ready to be used as an alternative to git yet.
We forbid long lived branches as they tend to deviate too much from the main branch.
It is up to the contributor what best suits their workflow
I disagree with this, each project has its own workflow, it’s up to the contributor to follow it.
with a ‘git rebase’ you’ll have to fix up the same conflicts over and over again
That is false, once rebased, you git push -f your branch and it is up to date with the main branch, you don’t have to refix the conflict (if you had one) ever again.
The problem is that solving conflicts is not shown in Github’s review UI (with either a rebase or a merge), and it is very easy to make mistakes there (or if someone really wants to, then hide something nefarious there)
IDE support for this is great, VS Code interactive rebase editor is good and we recommend using it.
If the commit history looks like a mess at the end of this then you can do “one” rebase at the very end before merging the feature brach to the main branch, with proper review and testing.
Rebasing a branch that has merge commits is when complex (old) conflicts happen and they are tricky and error-prone to fix. Whereas a daily git rebase keeps your work up to date and very often conflict free once your PR is ready for review.
When you have long lived branches (which you should not have), full of merge commits and only wait until the end to rebase, you’ll have to fix weeks old (if not months old) conflicts. The rebase will put your branch in an old state that you probably forgot about. That is too much error-prone, and I’ve spent days with coworkers helping them rebasing such branches, which led us to simply forbid long lived branches.
Anyway, this is orthogonal to the point I was trying to make: I find it annoying when people don’t follow the workflow you put in place. Whether you adhere to it or not should be irrelevant.
My initial comment was based on experience from having to maintain a feature branch that took nearly a year to get merged.
If (by policy) you have shorter feature branches than that, that’s great (the project I am working on is gradually trying to improve this too, now branches only live for months not years, and they have fewer conflicts).
I did have to solve the same conflict over and over again. Even though ‘git rerere’ might help, the person doing the rebase next time may not be the same one that did the rebase last time (e.g. if work is split in a team). With a very long lived feature branch like that master will move and you’ll need to rebase multiple times.
So you can either rebase a lot of times (daily is a bit too often, it’ll be very hard to unpick the bugs introduced on master yesterday from the bugs introduced on a feature branch yesterday, and you’ll end up spending all your time revising and triaging bugs instead of working on your feature, I did try to rebase at least weekly when possible) and risk introducing more bugs, or use git merge to reflect the actual development history of that branch. You are right that rebasing a merge sometimes doesn’t work either, and in that case it is better to leave it as it is.
A clean commit history would be nice to have, but retaining the development (and testing history) of a branch is equally valuable.
It is hard to impose a project policy that works for all situations, and I’d recommend to do it on a case by case basis. i.e. I am not suggesting that people should ignore a project’s rules, unless they have a good reason to (I.e. their workflow results in a better outcome for the project overall). In the end the contributor is the one doing the work, so they should pick whatever works best for them (with some guidance from the maintainers, although the maintainer may be less experienced if they never had to deal with a particular situation, like long lived branches before, and they may mistakenly recommend using ‘git rebase’ even in case where it is not needed and introduces more bugs).
TL:DR: listen to your contributors and try to find out why they are doing things differently, they might have a good reason (or indicate a systemic failure that should be addressed, e.g. management prioritizing things incorrectly in a way that is not compatible with a project’s policy, in which case either management or the project’s policy needs to change)
Git is missing git update to be used instead of git pull that would flip the order of branches. git pull is intended for maintainers who pull from outside to the tree they maintain. git update would be for people who work on a shared repository and need to merge their local changes into a common branch and then push it.
When a team is working on a shared branch or uses merge requests and prefers not to rebase long-running branches. It’s what anybody not working as a kernel maintainer expects to happen when you git pull with local work by default.
To be fair, I’m working with lots of repos and I’m getting really tired of everyone inventing their own style. I get where it comes from, I get why it matters to people who work on that specific project every day. But if I have a drive-by fix for something, every extra step just makes the contribution less likely. Especially if it’s not an automatically enforced rule. Atlasian annoyed me so much one day I just left a bug with a manually described diff for the change to avoid all the legal hassles their rules would cause.
If someone wants to fix a bug in my app and takes the one-off opportunity to submit it, I’ll take it even in a word doc.
GitHub’s UI does not allow us to review or comment on commit messages for pull requests.
I don’t understand this. You can comment on anything you like in a GitHub PR. Code review has a specific special UI, but people leave feedback on other things all the time as a normal comment.
I suppose it’s a bit awkward if you’re like, “I don’t like bef1482‘s commit message, can you remove the punctuation and fix the blah and make it 50 characters or less”, and then they do that, but now bef1482 doesn’t exist. When you comment on code, the context sticks around even when it gets force-pushed away, but that wouldn’t happen reliably for the commit messages. I think it’s a valid enough gripe, even though there’s nothing really stopping you like you say.
If you compare Github interface to e.g. Gerrit it’s easier to understand: I’m Gerrit the commit message looks like a pseudo-file and you can comment say, on line 2 of that commit message that you want something adjusted.
While you can read the commit message, and you can leave a comment that relates to the commit message, they’re not actually linked - you can’t comment on the commit message like you can a line of code in the diff. Tools like Gerrit include the commit message as part of the diff to allow this.
You can’t comment on lines of the commit message in diff view; but you can in just a regular comment say, “I don’t like Foo of commit bef1482” which will be automatically linked to the commit. I agree though that it could be helpful to have collapsable review style comments on the message itself that follow even after forced pushes/commit message re-writes.
No, if a PR has multiple commits and you want to review them 1 by 1, you can’t add comments on the commit. It makes sense to me because some changes in a commit can be overridden in a newer commit, so the comments would be on outdated code. I guess this encourages making smaller PRs.
For people who prefer rebase and force push, https://getcord.github.io/spr/ is a great tool resembling Phabricator’s arc diff. You just rebase and force push locally. The spr managed review branch will use merge commits to help GitHub’s PR system.
Having maintainers edit the PR description before clicking “Squash and merge” is useful. However, I feel that many project maintainers don’t edit descriptions.
We could in theory fix and cleanup the commits locally and manually exactly the way we do now and then force-push them to the remote branch and then use the merge button on the GitHub site and then they would appear as “merged”.
While I’m not a fan of bespoke cli tools - at least with gh cli you can avoid the “click button in UI”-step by way of:
You could even make a poor man’s Gerrit around this, if you stipulate PRs should contain a file named something like “COMMIT”, which you could then write a script to use for the commit message while deleting it, before continuing to squash/merge.
The timing of this is fascinating. Just a day or two ago, I encountered a similar situation on a completely different and unrelated repository, and was quite puzzled as to why my pull request had not been merged when I could see the commits in the repository.
It’s nice to have a bit of a “behind the scenes” glimpse at the thought processes of a high-level developer.
Honestly Github is so very much not fit for purpose in so many ways and yet here we are stuck in this lowest of local maximums.
I believe at least half of people are stuck there out of sheer laziness. There are other options. We just don’t want to deal with them.
Personally I’m not big on GitHub, and I avoid using it. Only if there’s a project that does, and I need to work with them. And I believe 90% of my usual (direct) interactions lately isn’t there at all.
Now, I know I’m probably an exception. And I don’t know why not many others would want to reduce g GitHub use as well. But they don’t and here we are.
One person’s laziness is another person’s optimal allocation of limited time resources.
What’s your go-to?
My own gitea instance :)
Not because it provides any superior experience - I simply don’t have any big-enough projects that would require something long-term stable. If I did, I would probably pick something like Codeberg or GitLab though, because in the past they never seemed to be so focused on doing everything, and they didn’t push “social” features that much and focused more on developers using the platform.
Now that I think of it, a lot of my reasons to avoid GitHub are just “old man yelling at cloud” type.
But I stand by the claim from my previous comment - most of us are just lazy to use something else. A lot of repositories don’t use any of the extras done by GitHub, so there’s no reason to use something else - except for “well everybody else is there, so we have to”. It takes like 3 minutes to clone a repo somewhere else then GitHub, and do your work there. But most people don’t even bother. And so GitHub remains almost a monopolist in this market. Which we all know how it ends.
You’d need capital to make something relatively smooth to use but now that the option space has been mostly explored anyway, no reason you couldn’t run the same track with a 10-20 person team.
Still you’d need a differentiator. I’m not sure whether novel technology (different vcs, nix/nx/turbo) or maybe novel process (native support for monorepo/trunk/patch development) would be it.
One possible big differentiator in 2024: “We don’t use your code to train AI models”?
Possibly, as added value: “Our admins have spent some time blocking the known scrapers.”
I’m not saying this without an amount of bitterness, but so don’t think that many people care that it’s a big differentiator.
Also - a recurring feeling seems to be that the vendors will eventually change their mind anyways.
It won’t be easy to prevent 3rd parties from scraping github or other code sites to collection repositories for AI model training. In the long run, if you are publishing your code, it will end up training AI models.
You’re right - but with a catch.
I live in a jurisdiction where there is legislation on copyright for ML and data mining. The EU TDM copyright exception says that while ML and data mining for academic research purposes can ignore copyright, commercial data mining and ML training has to respect a “machine-readable opt-out”. There’s a W3C proposal for a protocol that serves such a machine-readable opt-out to scrapers, and several other proposals for the technical side as well. I’d gladly host my code somewhere that had some competent people dealing with all that (perhaps with IP blocks for poorly-behaved bots).
Of course, more cynically: I fully expect the AI companies will ignore those opt-outs and steal everything anyway.
Another thing I’m missing in github PRs is iterations. I’m of the school of thought that git commits should be well-crafted tools of communication instead of periodic check-ins: basically, within a PR, diffs should be decoupled from the actual commits that will land in the target branch. So when people comment on my PRs I usually go back and modify past commits in-place with
rebaseinstead of adding new commits responding to their feedback. But then when I force-push these changes to the PR, people can’t see the actual changes I made in response to their feedback.Perhaps SourceHut is right and the answer is really to RETVRN to
git-send-mail. Federated stuff is really having its moment these days, and what’s more federated than email?They can, and over the years github has made it very slightly more visible (it used to be really well hidden), although it’s quite fragile: if you go on a PR where you forced-pushed stuff, in the history you’ll see something along the lines of
The (now underlined) “force-pushed” is a link to the diff for the force push, that is it diffs the HEAD of the PR before and after the force push. Now the big trap is if you rebase on top of a branch which has had updates then those updates will be part of the diff, but if you only rebase “in place” for the purpose of edition, it’ll work fine.
Kinda.
One bit that’s dearly missing is comments may or may not live across the change, and so relating the comments to the changes can be difficult.
Same with gitlab.
Oh man, I would hate that. I like to see a new commit on the PR because then I can look at just that diff to see if / how my comments were addressed and what the code looks like now instead of having to re-review the whole thing or sort through a larger diff for the small part I want to see.
I think what the the comment you’re responding to is describing is a system where you get these kinds of diffs, even if you use other workflows than adding a new commit per round of review.
I see what you’re saying, but solving that would require some kind of system to handle revisions, and isn’t that just, well, Git?
You can use fixups, ‘git-autofixup’ would even split a commit and fixup the correct commit in the history most of the time. Then when reviewers are happy you can rebase and squash the fixups only. You are right, this functionality is all built into git already…
It is, but it’s not the commit log.
On a system like Gerrit or Phabricator, each PR has a branch; previous revisions of a PR tell you which commit that branch pointed at sometime in the past; and there’s no requirement or expectation that earlier revisions are parents of later revisions.
This functionality exists in git via the reflog. And remembering that
git diffcan show the differences between any two arbitrary commits, you can useto see the changes between what your branch pointed at 4 revisions ago and what it points at now — even if there is no commit history in common between the two, because of rebasing, amended commits, etc.
Doing such a commit temporarily as a hack can be ok I guess. But there’s nothing worse than doing a bisect only to find that the bug was introduced in “fix pr feedback”
One thing I’ve found is that people who are very invested in using GitHub (or any other forge) can often conflate the unit of review and the unit of change.
For someone who isn’t familiar, comfortable, or interested in using git as a local tool in its own right, and only use it as a way to interact with GitHub PRs, it’s very easy to lose sight of the git commit as unit of change, and start to think of the PR as the unit of change instead. And that leads to all of the suggestions like “never force push”, “make lots of small commits to record your progress”, “don’t worry about using something bland like ‘fixed some stuff’ as your commit message”. I personally much prefer the workflow (and resulting history) that @ahelwer is suggesting, but I’ve at least come to terms with “not giving two shits” about the commit log being a self-consistent world view, even if it’s not one I agree with.
In combination with Gitlens to see previous line commits, I now make lots of commits with messages that look like “fixup: Prev Commit Title” and I push that. Once all feedback is addressed I make the rebase
Maybe you know this already, but git has built-in features for this:
git commit --fixup <commit-hash>andgit rebase --autosquashI didn’t know this actually. This is great!
You should use
fixup!as your prefix, it’s the one generated bygit commit --fixupandgit rebaseunderstands that syntax: if you invokegit rebase --autosquashit’ll automatically move the fixup commits after their subject.Also
squash!(if you want to edit the message of the merged commit), andamend!(if you want to completely replace the original commit message).Thanks for the helpful info. I had no idea!
Yeah I’ve tried that but then sometimes people jump the gun and merge all the commits as-is
Annoyingly git-send-email doesn’t actually transfer commits. It allows creating a similar commit with the same diff.
I would love it if you could actually send a commit by email, such that as long as you have the base commit you could reconstruct the same commit when applying. This way merges and other git operations work more cleanly as Git is aware that they are the same commit. This also makes it clear what the intended base is, sometimes it can be annoying to find where a patch applies cleanly.
I think
git-send-emaildoes that if you use the--baseoption.Thanks for sharing that flag, I’m going to use that from now on, listing the base is a nice bit of metadata for anyone trying to use the patch. In fact I am going to try setting
format.useAutoBase=trueand see if it breaks any automations that I use.However it doesn’t seem to perfectly construct the original commit.
git format-patch HEAD~ --base=HEAD~git checkout HEAD~git am 0001-Update-the-output-target.emlBut the resulting commit is different. It seems to just be the “committer” info that is different. Running a diff I see that the
committertag is different (current time, and probably updated to whoever I am) and thegpgsigis freshly generated. Both of these bits of information were never part of the original patch so it is clear that there isn’t enough information to perfectly construct the original commit.What I would like is to perfectly reconstruct the original commit, then I can create a merge commit or similar if I want to record who committed the patch to whatever branch it is becoming part of. Leave the original unchanged, but use a merge or similar as a record of how the original work was integrated into the project.
Ah yes, I forgot about the committer metadata. There’s the commit time as well as their name and email address.
git amhas--committer-date-is-author-datebut no option like--committer-is-authorIn practice ActivityPub & XMPP come to mind. Almost all users of email are using one of like 4 services where as these two protocols seem to have a lot more self-hosting & small instances relative to the large ones. Email sees more general usage so the spam prevention mechanism are high further incentivizing just moving to a provider that works with the Google & others.
Now extending to a real-time, collaborative, federated, MUC-per-patchset in a hypothetical
git send-xmppis something I would love to see. :)Bitbucket has handled this nicely for years, at least the version descended from stash. But evidently that’s not enough of a differentiator to encourage people to use it.
How does that work with git-send-email? You still get a new thread with all new patches after a change…
Basically I meant following the linux kernel patch review style, which probably means everybody uses mutt and is familiar with the concept of topposting and why it’s bad.
Sure, I understand the context – but how do you see only the “new changes” in this case?
I just do this. It makes contributors feel good, after putting in all that work to make a patch.
It’s not just about feeling good, it’s also a useful signal in the notifications.
A PR that’s merged you mark as read and carry on. A PR that’s closed you have to check to see why, and possibly change of plans.
The ratio of open / merged / closed PRs on a repo is also a useful (if weak) signal on odds of contributions being accepted.
It’s not even generally doable, as the contributor has to leave “allow edits from the maintainer” enabled, and that has never and still does not work when contributing from an organisation. But it’s only been five years and dozens of frustrated maintainers so…
Indeed, that is a thing Github can do internally (as you can “rebase and merge” or whatever via their green button and the PR will be marked as merged).
It’s been requested for more than a decade (also in keyword form), it was requested on the old community forums when isaacs shut down, and it was requested on the new discussions thing.
On a somewhat related note, I despair at the amount of people who does not bother to try to stick with how contributions in a project are done.
I started using Gitmoji to annotate my commits (IMHO it makes the git log far more understandable, and it’s less awkward to mix scopes compared to “conventional commit”). One would believe that when you send a contribution to a repository following a certain format for their commit messages, you would stick to that format as well. But in reality, it never happens, and on every PR I either need to force-push after editing the commit messages myself, or I need to ask the contributor.
Even when I put a
CONTRIBUTING.mdfile in my repository, the rules are not followed. On hobby projects I try to not get too annoyed by that, but at work, when I have to waste a hour 3 times a week explaining to a coworker that no, I won’t merge the PR until they follow the rules they did not bother to even look for, it gets on my nerves.NB: The Gitmoji is just an example, I have the same problem with the
git rebaserequirement instead of merging the main branch into your branch, or even the “you should update the test suite and documentation”.For short lived PRs ‘git rebase’ is fine, but for long lived feature branches you’ll make more mistakes with rebase than with merge. It is up to the contributor what best suits their workflow (and it doesn’t help if maintainers give conflicting advice on ‘git rebase’ vs ‘git merge’) If you fix a conflict the wrong way you will have lost from your history the “correct” commit (unless you can dig it up from ‘git reflog’). With a merge it is all still there and if you made a mistake fixing a conflict then you can see how the correct commit looked like and fix it up. Plus with a ‘git rebase’ you’ll have to fix up the same conflicts over and over again, whereas with a merge you won’t have to (unless the merge introduced new conflicts). If the commit history looks like a mess at the end of this then you can do “one” rebase at the very end before merging the feature brach to the main branch, with proper review and testing.
The problem is that solving conflicts is not shown in Github’s review UI (with either a rebase or a merge), and it is very easy to make mistakes there (or if someone really wants to, then hide something nefarious there)
As much as I don’t like patchqueues, it does solve the problem of commit evolution. You can always see the full history of a patch, and how it evolved over time, even if a mistake got introduced when rebasing the patch months ago, and you can see and comment on commit messages too. Unfortunately reviewing changes is very difficult, there is no good UI, and reviewing what is essentially a diff of a diff is very error prone, unless someone supplies a git tree with the patches applied for review too.
Pijul has some interesting ideas to solve these problems, but I’m not sure whether it is ready to be used as an alternative to git yet.
We forbid long lived branches as they tend to deviate too much from the main branch.
I disagree with this, each project has its own workflow, it’s up to the contributor to follow it.
That is false, once rebased, you
git push -fyour branch and it is up to date with the main branch, you don’t have to refix the conflict (if you had one) ever again.IDE support for this is great, VS Code interactive rebase editor is good and we recommend using it.
Rebasing a branch that has merge commits is when complex (old) conflicts happen and they are tricky and error-prone to fix. Whereas a daily
git rebasekeeps your work up to date and very often conflict free once your PR is ready for review.When you have long lived branches (which you should not have), full of merge commits and only wait until the end to rebase, you’ll have to fix weeks old (if not months old) conflicts. The rebase will put your branch in an old state that you probably forgot about. That is too much error-prone, and I’ve spent days with coworkers helping them rebasing such branches, which led us to simply forbid long lived branches.
Anyway, this is orthogonal to the point I was trying to make: I find it annoying when people don’t follow the workflow you put in place. Whether you adhere to it or not should be irrelevant.
My initial comment was based on experience from having to maintain a feature branch that took nearly a year to get merged. If (by policy) you have shorter feature branches than that, that’s great (the project I am working on is gradually trying to improve this too, now branches only live for months not years, and they have fewer conflicts).
I did have to solve the same conflict over and over again. Even though ‘git rerere’ might help, the person doing the rebase next time may not be the same one that did the rebase last time (e.g. if work is split in a team). With a very long lived feature branch like that master will move and you’ll need to rebase multiple times.
So you can either rebase a lot of times (daily is a bit too often, it’ll be very hard to unpick the bugs introduced on master yesterday from the bugs introduced on a feature branch yesterday, and you’ll end up spending all your time revising and triaging bugs instead of working on your feature, I did try to rebase at least weekly when possible) and risk introducing more bugs, or use git merge to reflect the actual development history of that branch. You are right that rebasing a merge sometimes doesn’t work either, and in that case it is better to leave it as it is. A clean commit history would be nice to have, but retaining the development (and testing history) of a branch is equally valuable.
It is hard to impose a project policy that works for all situations, and I’d recommend to do it on a case by case basis. i.e. I am not suggesting that people should ignore a project’s rules, unless they have a good reason to (I.e. their workflow results in a better outcome for the project overall). In the end the contributor is the one doing the work, so they should pick whatever works best for them (with some guidance from the maintainers, although the maintainer may be less experienced if they never had to deal with a particular situation, like long lived branches before, and they may mistakenly recommend using ‘git rebase’ even in case where it is not needed and introduces more bugs).
TL:DR: listen to your contributors and try to find out why they are doing things differently, they might have a good reason (or indicate a systemic failure that should be addressed, e.g. management prioritizing things incorrectly in a way that is not compatible with a project’s policy, in which case either management or the project’s policy needs to change)
Git is missing
git updateto be used instead ofgit pullthat would flip the order of branches.git pullis intended for maintainers who pull from outside to the tree they maintain.git updatewould be for people who work on a shared repository and need to merge their local changes into a common branch and then push it.I think it is spelled git pull –rebase
Nope, that performs rebase. It is spelled (more or less):
Which is kind of mouthful.
That looks like a merge of local master into remote master? In what circumstances would you want that?
When a team is working on a shared branch or uses merge requests and prefers not to rebase long-running branches. It’s what anybody not working as a kernel maintainer expects to happen when you
git pullwith local work by default.To be fair, I’m working with lots of repos and I’m getting really tired of everyone inventing their own style. I get where it comes from, I get why it matters to people who work on that specific project every day. But if I have a drive-by fix for something, every extra step just makes the contribution less likely. Especially if it’s not an automatically enforced rule. Atlasian annoyed me so much one day I just left a bug with a manually described diff for the change to avoid all the legal hassles their rules would cause.
If someone wants to fix a bug in my app and takes the one-off opportunity to submit it, I’ll take it even in a word doc.
I don’t understand this. You can comment on anything you like in a GitHub PR. Code review has a specific special UI, but people leave feedback on other things all the time as a normal comment.
I suppose it’s a bit awkward if you’re like, “I don’t like
bef1482‘s commit message, can you remove the punctuation and fix the blah and make it 50 characters or less”, and then they do that, but nowbef1482doesn’t exist. When you comment on code, the context sticks around even when it gets force-pushed away, but that wouldn’t happen reliably for the commit messages. I think it’s a valid enough gripe, even though there’s nothing really stopping you like you say.Awkward as hell is what it is. Reviewing commit messages is miserable.
If you compare Github interface to e.g. Gerrit it’s easier to understand: I’m Gerrit the commit message looks like a pseudo-file and you can comment say, on line 2 of that commit message that you want something adjusted.
Sample “PR”, although it doesn’t have any comments on the commit file: https://android-review.googlesource.com/c/platform/tools/security/+/3108165
While you can read the commit message, and you can leave a comment that relates to the commit message, they’re not actually linked - you can’t comment on the commit message like you can a line of code in the diff. Tools like Gerrit include the commit message as part of the diff to allow this.
You can’t comment on lines of the commit message in diff view; but you can in just a regular comment say, “I don’t like Foo of commit bef1482” which will be automatically linked to the commit. I agree though that it could be helpful to have collapsable review style comments on the message itself that follow even after forced pushes/commit message re-writes.
No, if a PR has multiple commits and you want to review them 1 by 1, you can’t add comments on the commit. It makes sense to me because some changes in a commit can be overridden in a newer commit, so the comments would be on outdated code. I guess this encourages making smaller PRs.
You can leave a comment on the commit if you’re going 1-by-1, but not the commit message
which leads to:
https://github.com/firebase/firebaseui-web-react/pull/173/commits/acb47b46dc39682d13f2b117524bda95ec1aeddf
Which goes away if anyone force pushes, as it’s tied to the sha, and not the change.
I haven’t seen this happen, I suspect you’re referring to general comments on commits
GitHub allows you to:
I haven’t seen comments on (2) lost after a force push, they should still available on the PR overview.
Oh I didn’t know, thanks
For people who prefer rebase and force push, https://getcord.github.io/spr/ is a great tool resembling Phabricator’s
arc diff. You just rebase and force push locally. The spr managed review branch will use merge commits to help GitHub’s PR system.I’ve also got lots of complaints when LLVM switched to GitHub PR: https://maskray.me/blog/2023-09-09-reflections-on-llvm-switch-to-github-pull-requests#patch-evolution
Having maintainers edit the PR description before clicking “Squash and merge” is useful. However, I feel that many project maintainers don’t edit descriptions.
While I’m not a fan of bespoke cli tools - at least with gh cli you can avoid the “click button in UI”-step by way of:
gh pr merge 123 --squashor something like it.https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request
I generally use it to deal with dependabot prs.
You could even make a poor man’s Gerrit around this, if you stipulate PRs should contain a file named something like “COMMIT”, which you could then write a script to use for the commit message while deleting it, before continuing to squash/merge.
The timing of this is fascinating. Just a day or two ago, I encountered a similar situation on a completely different and unrelated repository, and was quite puzzled as to why my pull request had not been merged when I could see the commits in the repository.
It’s nice to have a bit of a “behind the scenes” glimpse at the thought processes of a high-level developer.