[HN Gopher] Commit comments no longer appear in the pull request...
___________________________________________________________________
Commit comments no longer appear in the pull request timeline
Author : zdw
Score : 91 points
Date : 2022-08-07 13:51 UTC (9 hours ago)
(HTM) web link (github.blog)
(TXT) w3m dump (github.blog)
| donatj wrote:
| 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.
| ColanR wrote:
| I'd predict they have a plan to replace it with some GitHub-
| specific metadata. That would enhance lock-in and follow the
| usual Microsoft strategy.
| donatj wrote:
| Commit comments already are GitHub specific metadata.
| IggleSniggle wrote:
| 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.
| kadoban wrote:
| Oh we're not talking about commit messages? Wtf are
| commit comments then?
| msbarnett wrote:
| 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.
| kadoban wrote:
| Oh. Thanks.
|
| If they're inside of PRs are they still commit comments?
| I'm thinking not, in which case maybe I never noticed
| those were a possibility.
| avar wrote:
| 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.
|
| E.g. here's git.git's first commit, with a lot of random
| (mostly garbage) comments:
| https://github.com/git/git/commit/e83c5163316
|
| 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.
| rwmj wrote:
| 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.
| eurasiantiger wrote:
| 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.
| rwmj wrote:
| 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.)
| FeepingCreature wrote:
| Depends how long their tail is, I guess.
| untitaker_ wrote:
| 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.
| momothereal wrote:
| 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.
| bastawhiz wrote:
| 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.
| swayvil wrote:
| >It's really hard to understand the context of these comments.
|
| You, sir, have been caught. At the time of this writing there
| is only 1 other comment.
| kemayo wrote:
| 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.
| swayvil wrote:
| fair point.
| bastawhiz wrote:
| I'm talking about the comments that GitHub is removing from
| the timelines.
| exhaze wrote:
| No explanation about why.
|
| No name for blog post author.
|
| No gradual rollout I think? Was this ever a beta feature first
| like most other things?
|
| Compare to Stripe product releases where people who built it are
| instantly on HN, responding to feedback.
|
| Is there someone at GH who actually owns the PR experience and
| sets the long term roadmap for non-enterprise?
| exhaze wrote:
| 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.
| bmitc wrote:
| 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).
| politelemon wrote:
| I didn't know about this, that's great. And it's an intuitive
| URL, github.com gets replaced by github.dev.
| codeapprove wrote:
| 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
| hedora wrote:
| 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
| corytheboyd wrote:
| 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.
| theK wrote:
| 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"
| corytheboyd wrote:
| Yeah totally agree, just make it work like every other
| comment
| swayvil wrote:
| Comments are arguably even more important than the code. This is
| a wonkass move.
| marcinzm wrote:
| If it's that important it should be a comment in the code
| rather than one randomly floating in GitHub.
| exhaze wrote:
| 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.
| marcinzm wrote:
| >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.
| ryanbrunner wrote:
| 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.
| shawabawa3 wrote:
| 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
| [deleted]
| MisterBastahrd wrote:
| 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.
| peterkelly wrote:
| This seems like a really bad idea.
|
| 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.
| drey08 wrote:
| This is about commit comments, not commit messages.
| peterkelly wrote:
| Ah, thanks for the clarification.
|
| This was definitely not obvious to me from their announcement
| and I wouldn't be surprised if others similarly misinterpret
| it.
| politelemon wrote:
| It was very obvious to me, it's right at the top of the
| page.
|
| _> Note: This change does not impact regular review
| comments or other comments made directly on a pull
| request._
| rpdillon wrote:
| 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.
| hinkley wrote:
| 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.
| vbezhenar wrote:
| 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.
| doodlesdev wrote:
| 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.
| emailtho wrote:
| 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.
| IggleSniggle wrote:
| 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.
| layer8 wrote:
| GitHub used to have some support for notes, they should bring
| that back: https://github.blog/2010-08-25-git-notes-display/
| bayindirh wrote:
| That's called a mailing list, and it's supported by Git, no?
| hinkley wrote:
| 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.
| bayindirh wrote:
| 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.
| procrastitron wrote:
| You can do that with git-appraise:
| https://github.com/google/git-appraise
| gmfawcett wrote:
| Fossil (https://fossil-scm.org/) seems to align with your
| ideals.
| xSxY3fj5gVCmvWE wrote:
| Can someone elaborate or attach relevant links? I'm extremely
| interested.
| doodlesdev wrote:
| Relevant discussion: https://www.reddit.com/r/linux/comment
| s/914z8v/comment/e2yib...
|
| I do find the homepage very complete and the documentation
| very understandable though, would recomend taking a look if
| you're interested.
| rodgerd wrote:
| 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.
| ISL wrote:
| Pulling @drey08's message up to parent-level:
|
| "drey08 38 minutes ago | unvote | parent | next [-]
|
| This is about commit comments, not commit messages."
___________________________________________________________________
(page generated 2022-08-07 23:01 UTC)