Oh that’s super annoying. We use that to draw attention to important things that changed in certain commits, for instance we attach human readable diffs to lock file change commits.
Beyond that sometimes it’s super helpful to add comments as a human directly to a commit noting details about that specific change set that don’t make sense in the commit itself.
This is one of the most boneheaded moves I have seen GitHub make.
It does seem very strange. If I'm looking at a PR, especially a historical one which has been merged already, I want to see all the comments which were made about the commit in one place.
If it was confusing to have these comments turn up "out of context" then maybe that's something that github could have improved instead, by visualising the context better.
They seem to be removing heavy features from often-used pages. I would imagine this change creates quadratic savings in data fetching for the timeline.
Honestly, if that was the reason ("we need to make our mysql faster"), I would strongly prefer if they'd just say that. Certainly more understandable (if not relatable) than this sort of communication.
They've been in a roll nerfing stuff like this ever since MS bought them. See "load more comments" ajax links when there are more than 10 comments in a PR review, or "load diff" when a file has changed more than a few lines. All of these degrade the ability to actually look at changes and review them, which is kind of the whole point of pull requests. But alas, the enterprise must enterprise.
This could be so, but wouldn't a better solution be to prerender popular pages or cache generated HTML (a la memcached etc)? (Having said that I have no idea how modern web apps work, nor what technology stack github is using.)
Caching what? All possible comments everywhere? That’s just another db, not a cache. And having infinite caches is not feasible, the point is to store widely and frequently accessed data, not anyone’s comments on any commit in the repo.
I see I wasn’t the only one to conflate “commit comment” with “commit message.” For a good 5 minutes I thought this was saying that GitHub would no longer show commit message content in the pull request timeline, which does seem like a crazy vendor lock-in scheme.
When you view a commit on GitHub, you can attach a comment to any given line of it. It looks basically like a comment left on a line of code in a PR, except tied to a commit instead of a PR.
A comment <-> PR is a many-many relationship. For a lot of workflows it's practically a one-to-one, as people would only comment on a commit starting from a PR, but you can just comment on any given commit you find.
That wasn't part of a PR (GitHub didn't even exist then), and if it was it could be a part of many different PRs.
I really don't have a full overview of this GitHub change, but this general area is something other hosting providers have definitely struggled with.
I.e. how and when to treat a PR/MR as some holistic vertical component, v.s. being mostly incidental metadata about a "push" (or "potential push"), with the commits (and any comments) being the important way to view or think about individual changes, and anything in-between.
You can still add comments to an individual commit and view them from various pages. However, comments added this way will no longer surface in the timeline(s) of pull request(s) that happen to include the commit. This does not change anything about pull request review comments, including review comments added when reviewing a pull request commit-by-commit.
To balance out my parent negative comment, I think the semi recent release of browser VS code on GH web (just press “.”) is awesome. Just wish the PR review experience would get more love.
Damn. I had no idea about that feature. That is awesome.
Although, it doesn't seem to recognize my extensions, even though it automatically logged me into my VS Code account via GitHub's account (which is how I normally do it).
Shameless plug but at CodeApprove we’re pretty obsessive about giving the GitHub PR review experience “more love”. Check out https://codeapprove.com if you’re interested
Does this help? We hope you'll accept our pull request. ;-)
And yes! We have engineering, product, and design leads supporting the PR experience who are super interested in getting your collective feedback about where we should take the experience next.
I want a dvcs that stores code review comments directly into the repository.
We need the feedback loop and the historical setting for why we made this decision two years ago. Until or unless we treat requirements as first class citizens in our source code, archeological digs are the last best hope for avoiding reintroducing old bugs.
I also need to be able to go back and disagree with something I or someone else said as an absolute in a commit message that turned out not to be an absolute. That stuff trips up the new people. It’s the worst kind of tribal knowledge.
The idea that Fossil has as an SCM is that all your workflow ought to belong in the source repository; so, for example, the issue tracker that comes with Fossil stores all of its data inside of the same repo as the code. If you work on an issue and update it, the update travels with the code commits. If you raise an issue and push it to an upstream repo, everyone who syncs against it will see the issue, and so on and so forth.
You could do them as git notes, which gives you a non-vendor specific mechanism for handling this kind of data in git. However, you do need to make a point about having changes to notes get pushed to version control, so it does require either a tool or onboarding to become useful for what you’re talking about.
Git can store a variety of other objects in the repo as well, but the built in git-notes mechanism springs most immediately to my mind as the thing you’re looking for.
Comments can be made specific to a code line. Git notes are attached to a SHA-1. So you would first need some standard how to refer to lines or nobody else can read your comments.
What about commiting those comments directly into source files as comments with some tag? Like `// REVIEW looks like a bug`?
Actually I think that lots of things that's currently located outside of the repo should be in the repo. Like issues: just put them into some `issues` directory with some standard format and build UI around it.
That's basically what Fossil does. You get to do distributed issue tracking and wiki using it, it also has some advantages when it comes to subprojects compared to git submodules.
I like that idea and have tried that approach. Unfortunately, there needs to be tooling that can more easily parse a repository for those and make them part of the flow. Simply grepping isn't really a great way to do that.
That is an ungoogleable concept so unless it’s front and center in the docs or posted here every six months, nobody is going to know what you’re talking about.
Do you mean attaching a mailing list to git or a feature for syncing comments to commit ids? If the former, you think sending a new guy to read four years of yammering about code is a useful or non hostile suggestion? It’s okay to be a misanthrope, as long as you understand that about yourself and when it colors your interactions.
But this is how this conversation usually goes:
There are any number of addons and special hidden features of git. If they don’t propagate to and from developer machines without proper user configuration, it’s a checkbox feature, not a real one. And the worst kind - the ones where people can dismiss a real and near universal problem by saying “we have X at home”.
Half of the good features of version control, CI/CD, you name it, are good in part because they only require one or two people to put in the effort and then everyone benefits, whether they recognize the value in it immediately, over time, or leave the team insisting it never helped them once even though other people know that’s not true.
Any feature that requires everyone to set it up on their box requires unanimous decision. That doesn’t scale, and it barely works on a small fixed team either.
Most of the tools I see people claim git has involve changing the configs or your behavior in order to sync the extra metadata. Do it by default or don’t do it at all. This is you being wishy washy and letting someone else take the blame for it not being used.
Edit: are you talking about ‘git am?’ That has nothing at all to do with what I’m talking about and also is only used on non commercial products. The only people I know of who use this at work are professional OSS committers who aren’t working on projects hosted on GitHub. Which is zero people actually in my professional circle, and if those people know anyone they haven’t mentioned it where I’ve heard it.
Yes, I'm talking about "git send-mail" and "git am". Actually, it's relevant to what you're already saying. You can have an e-mail list based code review process, and store it in your on-premise servers, or in your infrastructure for what matters.
You can argue about the utility and practicality of GitHub, and the tools provided by the platform, however Git is neither is an invention of GitHub nor have a monopoly over it.
Lastly, having no clue about a technology as a professional developer has nothing to do with the utility or usefulness of the feature. On the contrary, a professional developer should be able to work in a much more flexible and open-minded manner about different working methods and workflows.
All in all, the mailing list can act as an internal time-based knowledge base which can store both the code review and decisions coming out of it. The decision to use it or not of course yours and your team's, but send-email and am are not features confined to "professional OSS committers who aren't working on projects hosted on GitHub" by any means technical and social.
Also, you can like the feature or not, and may decide against it just on personal preferences. However, this neither moves the feature to a niche place, or makes it obsolete.
The mailing list does not tie directly to a line of code. That’s the point of tracking commentary with the code. It helps keep a rough level of confidence about what comments are still relevant (based on whether or how much of the code survived).
Email allows inline git reviews and comments, without HTML, CSS, JS, and the microservice jungle needed to host a web app these days.
I follow a few of the Linux kernel mailing lists and frankly I am jealous af; no Slack, no Jira, no web UI I can accidentally refresh which flushes state and deletes the comment I was writing. Was less bandwidth hungry.
Hosted UI just seem like a huge waste and a whole lot of agency capture by VCs. Just release a Docker image, give me an API key.
Same here. I've tried though for some time to get more into the Linux Kernel workflow of git and struggle to this day. I think it is great for people that have spend the time to incorporate the built-in tooling, but a lot of people simply never because it is time consuming and opinionated. The biggest hurdle is probably just having a text-based mail client and SMTP setup.
It was very confusing having commit level comments show up in the PR feed along with PR comments. IIRC you couldn't reply to commit comments from a PR (only from the commit view) but you could reply to PR comments, which was weird. Seems like a good move to me, but it's unclear what happens when someone replies to an email from GitHub for a commit, does it just go nowhere? Does it go nowhere unless the commit is associated with a PR? What if that PR is closed/merged? Wish this post shared a bit more about what's going on behind the scenes.
I don’t get it. From a UX perspective the common expectation would be to be able to see all discussion related to the commits in the PR. If the sources vary give visual clues. It lies close to arrive at “If the user can interact with one comment from a PR view they should be able to interact with all comments, doing otherwise would inhibit discourse”
This is fine to me, tbh. It's really hard to understand the context of these comments. If you're not reading the code, it makes little sense to see the comments on that code. You can't even review a PR from that view, so having them in the timeline doesn't actually help prevent mistakes. For long PRs, the timeline gets very cluttered with comments and becomes almost unusable.
Amusingly enough, unless this is your joke whooshing over my head, in this case you're the one not understanding the context of a comment. OP is talking about the same comments the article is talking about -- those on commits on GitHub.
I put a lot of effort into commit messages and try to always give a good summary of what (and more importantly why) the change was made. I consider it almost as important as the code itself, because it serves as a form of documentation, as well as a way of communicating to both other team members and my future self.
I struggle immensely with writing normal documentation, whether it be fine-grained stuff like descriptions of functions/classes, or higher-level architectural issues (I have written a PhD thesis so I can do it, it's just a tremendously difficult undertaking). I treat commit messages like a second best alternative to code documentation, and they're a lot easier to write because all the information is fresh in my head at the time I make a commit. A good log can also serve as a solid starting point for later writing other forms of documentation.
Git is designed as a decentralised version control system; pull requests are something that the major repository hosting providers have added, and while I appreciate the existence of those features I'd rather not rely on them at least in terms of the long-term record for a project. In the case of pull requests, someone who is already writing detailed commit messages basically has double the work if they want to communicate the same kind of information in pull request descriptions/comments, though you can just make the comments a copy of the log (BitBucket does this). And if you want to migrate from one provider to another you can lose all this extra context that is not in the repository itself.
Other than attempt to increase lock-in, I don't understand why they would make this change, and it seems like it's based on assumptions about how some teams use git without taking into account other styles - there's considerable variety in practices between teams and for those that rely on the commit messages more heavily this seems like it's going to make for a worse experience.
I had the same confusion as gp and spent a couple of minutes resesrching to figure it out. The reason the above quote didn't help me is that is clarifies that this isn't about review comments or other PR comments. But commit messages are neither of those.
Root cause? I didn't even know commit comments were a thing that GitHub had! I assume it was much clearer to everyone that knew about them.
How do you test that the commit messages are accurate over time for what the software does?
Anytime Ive done code archaeology, the commit messages, no matter how detailed, have been irrelevant to what's currently running.
Between reading a book of commit comments, or the authoritative code, it's much more efficient to just read the code.
One of the big things with commit messages is that they won't capture changes in adjacent systems that don't share a code base. While you can spot weird edge cases in the code, you can't from the commit messages alone
The point is to capture what is true at the time the commit is made, and in aggregate to tell a story of the code's evolution. If someone wants to know why a particular thing is done the way it is, then this can provide insights as to how it got that way and why.
I don't regard a commit log as an adequate substitute for documentation on the latest version of a codebase. Rather, it can provide a useful starting point for writing documentation that I find easier than beginning from a "blank page" so to speak. It's a starting point only - there's a lot of work required to get from that point to an accurate description of the most recent version of a codebase.
I agree with your point about changes in adjacent systems, though where that happens I try to mention those changes in the commit message and explain how they relate to the commit itself. I'm speaking mostly from experience working on software that exists in a single repository; for things that are split among multiple independent repositories/services I agree it's more difficult (and this is why i prefer monorepos whenever its viable).
The amount of time spent reading and understanding something should be proportional to how controversial it seems at face value. Half the comments here clearly did not spend more than 30 seconds reading or understanding the changelog.
I hope GH fixes the notification link to PR updates. Right now it brings you to the latest commit, and if you comment on that page, it creates a commit comment instead of a PR review.
I wish they would add a project-wide configuration check box that creates a merge commit (with comment containing a PR number) when rebasing a PR to main.
Currently, you can only pick two of the following three things, which pretty much everyone wants. People have been asking for the above feature for 5-10 years.
- leave a pointer to the code review in the git commit log
- have a linear, bisectable history
- preserve developers' commit messages and intermediate work on branches
There are different types of comments in GitHub, and ironically, this change was made to reduce confusion by not surfacing comments made on individual commits (comments added from the commit page on GitHub) in the timeline(s) of pull request(s) that happen to include those commits.
Nothing has changed for "pull request review comments" (comments added from the pull request page, including comments added when reviewing a pull request commit-by-commit). Also, nothing has changed in the way GitHub surfaces commits or commit messages in the pull request (or any other) page.
Gitlab allows you to comment any line in the code even if it's not part of a MR now. In theory I find that highly useful, because you might have thoughts, findings etc. whenever looking at code, not only in a MR.
However, I have not found a way to discover any such comments later in a systematic way.
Not the only change. Something what caught me off-guard yesterday: creating a fork forks the main branch only by default. There's a new checkbox that has to be unchecked to create a complete fork.
After having the luxury of reading all the comments in this thread, it looks like it might be a tempest in a teapot. Apparently 99% don't even use this feature. So keep that in mind before you get alarmed.
This is a weird niche feature that I'm guessing 99% of GitHub users have never seen
Certainly almost every comment on hacker news hasn't understood the change
You can make comments directly on commit objects in GitHub, almost nobody ever does this. You used to see these comments appear PRs (but couldn't reply or resolve them) but now you don't. Nothing has changed with actual PR comments, or code comments, or anything that most people actually use
Also, it's a sort of "legacy" feature. Earlier versions of the PR UX made it tough to review commit at a time instead of just the Full PR Diff Firehose. (Now you can pull changes apart into commits or "updates" and several things between directly in the PR interface.) Commit comments and those automatically floating into relevant PRs was basically the old UX flow workaround to that. It was slightly less of a niche feature then. Now the PR UX supports those use cases more directly and there's much fewer reasons to use commit comments.
There is “should” and then there’s what people actually do. Do you suggest that parent commenter should now go and change their policy on comments to solve this real immediate issue? If not that, do you have practical advice for parent?
Parent commenter and several others said they found it very valuable. People seem to want this. Feels like enough of a reason to not remove it for all users with no heads up.
I don't think there's widespread usage of GitHub comments on commits as a code documentation method, since it would be almost entirely unusable (you wouldn't be able to see those comments unless you navigated to the specific commit on Github).
The use case I do see sometimes is having a pull request made up of distinct commits which are each commented on, but that's already a bit of a poor UX in GitHub, easily worked around, and is pretty uncommon.
>Do you suggest that parent commenter should now go and change their policy on comments to solve this real immediate issue?
I mean what is your suggestion to OP? Putting their head in the sand and acting like Github didn't remove a feature they in fact had removed? It's a very niche feature (so much so that more comments seem to misunderstand the feature than actually miss the feature) so the chance of Github not removing it is fairly low even if complaining causes them to delay it a bit.
That'd certainly be one way to get me to stop using github. Devs supply commit comments because they are useful. Taking away a useful thing because some suit sees them as superfluous is stupid. I wonder if they're gonna bring them back as a "premium" feature.
Beyond that sometimes it’s super helpful to add comments as a human directly to a commit noting details about that specific change set that don’t make sense in the commit itself.
This is one of the most boneheaded moves I have seen GitHub make.