Conversation

Lorenzo Stoakes

Another case where detailed commit messages really really help.

It blows my mind that any project decides not to provide detailed commit messages when the benefits are so massive...!
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1899ad18c6072d689896badafb81267b0a1092a4
3
10
15

@ljs 💯 agree. It's always surprising to me how a lot of github based projects don't insist on writing a detailed commit message. The reasons I have always been given is that people can go and read PR descriptions etc. Probably it's just the workflow some communities/developers are used to but personally I like to use git blame a lot to check the history of changes and understand the code etc.

0
0
1
@ljs How's your experience of commit messages in private repos of your (past) employers, btw?

In my experience, it's almost always much worse than the average open source repo, and nobody (who has a say in it) cares. Tends to irritate me.
3
0
2
@liskin sadly it wouldn't be appropriate for me to comment on that!
0
0
2
@janriemer @chriskrycho Spot on!

As I'm (nearly done) writing a book about mm to learn mm deeply, I totally buy the 'write about it to understand' thing, as I've experienced it first hand, endlessly.
0
1
3

@liskin @ljs I suppose if you mostly use github to browse the codebase, you can rely on PR descriptions

2
0
0
@6d03 @liskin commits are smaller granularity than PRs and different scope, that doesn't work at all.

It's just laziness it's quite simple
1
0
0

@ljs @liskin depends. at $work we squash all commits on PR merge, so it's the same granularity for us.

1
0
0
@6d03 @liskin trying to be more zen in 2024:

I respectfully disagree with this practice :)
2
0
5
@6d03 @ljs Well…
1) I don't mostly use GitHub to browse the codebase, it's too sluggish even on a superfast laptop that executes javascript in an instant
2) why do you think the PR descriptions are considerably better than the commit messages (spoiler alert: they are shit)
1
0
1

@ljs @liskin it's certainly debatable, but it does simplify life in the sense that people don't have to worry about cleaning up their commits, which in my experience can get tedious.

2
0
0
Edited 28 days ago
@ljs @6d03 Yes, squash-merging PRs is heresy.
0
0
2
@6d03 @ljs People not worrying about cleaning up their commits is exactly how you end up with shitty commits. And, in practice, those same people don't produce meaningful PR descriptions either.

Most of them (obviously at this point it would be silly trying to generalise to *all*) just suck at textual communication in general.
0
0
2
@6d03 @liskin yeah this is the 'argument', maybe I'll be less zen and tell you it's just terrible. It's not debatable, it's terrible.

Why are you even using source control?

You end up with a 10,000 line commit? No bisect now, no ability to back port, no description of the logic behind things, no separation of moving code/changing it.

I've seen somebody do a 5,000 line move of code and subtle changes in it - good luck dealing with that as one commit.

You remove all code-specific context, all things that are sub-PR level.

All for what purpose? To make life a little easier or neater?

If people can't be bothered to rebase properly, then why on earth do you think they'd put every bit of detail needed in a PR description?

It's impossible to argue these things anyway, and pointless, because those making the decisions are not the ones dealing with the consequences of this.

I've literally run into every single one of these issues in practice.

And you're commenting on a post where I literally point out the benefit of a high-granularity comment that wouldn't exist in this scheme?

Come on man.
2
0
3

Lorenzo Stoakes

Edited 28 days ago
@6d03 @liskin To reiterate - this stuff isn't arguable, it's laziness. Plain and simple.
0
0
3
@6d03 @liskin to be clear I"m not attacking you at all :) just have strong feelings on this issue. I'm just attacking the auto-squash concept as an idea.
1
0
3
@6d03 @liskin I realise that when one becomes passionate about such things it can read terribly... :)

It's also fine to disagree obviously.
1
0
0

@ljs @liskin @6d03 As a third-party observer, the insistence that this is not debatable does come across as rather attack-ish. (or, at best, intolerant) Aside from that I agree with your position 🙂

1
0
0

@liskin @ljs
(ignore this if you're not interested in random other people's input)

FWIW I have worked with various projects with various committing conventions at work. In some of them, commit messages are part of the code review and the commit messages in those projects are generally decent. In others, commit messages are not reviewed (those projects tend to use squash-and-merge) and unsurprisingly the typical quality of their commit messages is absolutely abysmal, definitely much worse than any open-source project I've been involved with.

0
0
0
@diazona @liskin @6d03 it's not debatable from my perspective because the arguments for auto-squash are extremely weak in comparison to against.

The facts of a matter might make something seem 'attack-ish' but I think it's actually worse to boths-sides-ism something that doesn't actually have 2 sides.

Society is too full of people who are desperate to pretend that everything is 50/50. It's not, sorry.
1
0
0

@ljs @liskin @6d03 I'm not desperate to pretend that this is 50/50. Frankly, I'm insulted that you would make that comparison based on my toot which said nothing of the sort.

Anyway, the point is that even though the arguments for auto-squash are weak compared to the arguments against, that doesn't make it not debatable. You can have a debate that one side wins in a landslide. Saying that this is not debatable effectively comes across as an attempt to reframe it, from a situation where one position can win out on its merits, to a situation where one position wins out because you decree that it should. I believe that's not what you intended, but that's the way it sounds. And I find that problematic. (I'm not denying that there are *some* situations where one side of a debate doesn't deserve to be considered for one reason or another, I just don't believe that Git workflows are one of those situations.)

2
0
0
@diazona @liskin @6d03 ok so let me tell you my approach to social media these days.

Life is too short, so if somebody's obviously in bad faith I just block them yeah?

You've reply-guyed a conversation you weren't invited to, but that's fine this is public.

But you've been hugely aggressive from the start and now you're reading things into what I've said that even you seem aware are fucking ridiculous.

Look at the top of this bloody thread and what I sincerely tooted about that you're now shitting all over in effect.

You have about 5 minutes to prove you're in good faith or I'll just block you. Cheers.
1
0
0
@diazona @6d03 @liskin okay doke, time's up, you've taken up enough of my Sunday. Bye!
0
0
0
@diazona @ljs @6d03 It's probably helpful to read "non-debatable" as "not worth debating for long, and the little time it was worth debating for has already been spent, so now it's not worth debating at all". Which is pretty close to what Lorenzo meant, and is consistent with your view of dabatability as well, I believe.
0
0
1

@liskin @ljs Do you have any pointers to best practice guides?

In my corporate auto-squash scenario I tend to write fairly detailed PR descriptions, which I then copy into the merge commit 🤷

Some, though not all of my coworkers apparently also do this.

2
0
2

@liskin @ljs

I agree, having all the relevant information in commit messages would be ideal. But what do you do when your project already has its established workflows?

1
0
1
@6d03 @liskin you're a good man for making the effort!

While I (obviously) disagree with auto-squash taking the time to really be detailed in what place you possibly can already adds a ton of value.

I think the guidelines at https://www.kernel.org/doc/html/latest/process/submitting-patches.html are relevant even for other projects (in the 'describe your changes' bit). Obviously tailor accordingly.
0
1
2
@6d03 @liskin Great! Sorry if I was rude, or worngly assume you were advocating for this. You can't help it if existing guidelines differ from the ideal, none of us can!

I think as per the other post, if you are putting in as much detail as you possibly can on the PR and given that is the level of granularity, that's already huge and you're doing god's work.

See the ref I posted there to kernel approach, quite good general approach to it. Obviously adapting to taste!

(and yes I'm biased towards kernel approach somewhat I must confess!)
0
0
2