0003321: Adopt a 50/72 and one-change-per-commit (merges) style for Mercurial commit messages

Notes
(0018678) Blzut3 (administrator) 2017-10-30 04:28	If we were a bigger project with multiple simultaneously maintained stable branches these kinds of things might provide a benefit. But since we're not the only thing they do is make it more annoying to work on. I don't know what tool you use to review changes, but I disagree on multiple commits being easier to review. They may change the same part of a file (unless you split your work into multiple commits after the fact) so the only sane way to get a complete picture of a multi-commit change is to flatten it. This is why Torr prefers single commit pull requests for Zandronum. As for the static word wrapping. TortoiseHg only shows a line at 80 characters and doesn't show the character count so definitely can't be bothered there. In the case of the commit you mentioned I was looking at how lintian reported the ldconfig problem for different prefixes and noticed that the prefixes weren't applying to the deb files. So I fixed that simultaneously while working the ldconfig trigger. Sure I could go and separate it into two commits, but it's not worth my time. So with a notable exception for typo fixes (which if we had branches to cherry pick into I would agree should be a separate commit) almost every commit from me is done at the conclusion of whatever the first thing I was working on. I then just itemize what was done while doing that thing. I'm leaving this open in case Zalewa wants to add anything or disagree, but this is a pretty strong rejection from me.

(0018680) WubTheCaptain (reporter) 2017-10-30 04:59	Quote from Blzut3 If we were a bigger project with multiple simultaneously maintained stable branches these kinds of things might provide a benefit. I argue it brings benefits even on default branch (master branch in git). There's also a consideration of Mercurial-Git bridges, where the first line (title) of the commit message is used for git-send-email(1) etc (sharing patch files). Quote from Blzut3 But since we're not the only thing they do is make it more annoying to work on. I thought so too first about git, but it has actually helped my workflow. For every feature I write: I keep a seperate feature branch, commit often, squash as necessary, then merge it (usually fast-forward in git so there's no merge commit). If I want to work on another feature, I switch branches and continue there. Rebase to HEAD of master (tip of default) to avoid merge conflicts, for any unpublished changes. Quote from Blzut3 I don't know what tool you use to review changes Mercurial on GNU/Linux. hg log --patch Quote from Blzut3 TortoiseHg only shows a line at 80 characters I'm not sure at all on this, but it seems to be configurable (long summary boolean option):'https://tortoisehg.readthedocs.io/en/latest/settings.html#workbench [^]' Quote from Blzut3 This is why Torr prefers single commit pull requests for Zandronum. Right before I posted this, I tried hard to find that recent comment from Zandronum tracker where Torr had commented on Leonard's(?) patch. I can't find it anymore 😢, but I felt like it was a compliment for multiple commit pull requests. Quote from Blzut3 In the case of the commit you mentioned Nothing personal, it was just the most recent commit to make an example of.

(0018681) WubTheCaptain (reporter) 2017-10-30 05:09	Quote from Blzut3 I disagree on multiple commits being easier to review. They may change the same part of a file (unless you split your work into multiple commits after the fact) so the only sane way to get a complete picture of a multi-commit change is to flatten it. I'm not convinced on this argument. I uploaded mercurial-merge-flow.txt, which shows a merge commit. This merge commits shows the "flat" version with hg log --patch. To visualize the tree: $ hg log --graph --style compact @ 3[tip]:0,2 652c87bd2e18 2017-10-30 05:06 +0000 wubthecaptain \|\ merge commit \| \| \| o 2 ad30688f99b3 2017-10-30 05:05 +0000 wubthecaptain \| \| added exclamation mark \| \| \| o 1 2c93680f2e3c 2017-10-30 05:05 +0000 wubthecaptain \|/ added "world" to hello world \| o 0 9423171a5403 2017-10-30 05:04 +0000 wubthecaptain hello world

(0018682) WubTheCaptain (reporter) 2017-10-30 05:43 edited on: 2017-10-30 05:52	I installed tortoisehg 4.3.1 to see how it works. Quote from Blzut3 I don't know what tool you use to review changes, but I disagree on multiple commits being easier to review. Oddly enough, tortoisehg doesn't show the diffs with merge commits in the main window until you click "Browse at Revision" right click context menu option. Quote from Blzut3 TortoiseHg only shows a line at 80 characters and doesn't show the character count so definitely can't be bothered there. Seems like the summary line length is configurable. Visual Editor and CLI Editor are also configurable to do the line wrapping for you. (Notepad++, vi, emacs, whatever you'd like.) I think? Curious to know what workflow/tools Zalewa has/uses. Personally, I found tortoisehg to slow me down with unexpected behavior (consider I'm completely new to both Mercurial and tortoisehg, but familiar with git).

(0018714) Zalewa (developer) 2017-11-01 15:09	Revert is not only overrated in arguments such as these, it is also a tool that degrades over time. It becomes gradually more difficult to perform depending only, and only on the age of the commit and not on any other factor. How often do you really revert anything? The initial change was made for a reason and when things don't work out the way they were supposed to, you fix the current code instead of going back to the old one. Reverting is like hitting a panic button, why would you do that? Plan your change beforehand and notice when code goes awry even before you make the first commit. If you need to partially revert some old change, it either can be done manually or through 'git revert bigchange; git reset HEAD^; git commit' (and in Hg - by using whatever clumsy equivalent it has). When your commit is old, you will drown in conflict mess regardless of how elegant this commit was. If you create multiple intermediary commits during work, all closely related to each other, squash them before pushing. That said, revert is useful to temporarily disable significant part of debug code that you want to use for the period of the work, but it affects the runtime singificantly and you need to disable it to perform proper/final testing. Furthermore, on the topic of what should the commit consist of, I'm all in for keeping commits as self-contained to singular change if possible. If I work on something and discover something else that also needs work, I prefer to do the side-job, commit it and then return to the original task. While this approach is in accordance to the unsung "best practice" standards, I mainly do it because it gets the "done" part of the job "saved" in the repository and it no longer distracts me when viewing diff on current changes. I also know I won't lose it if I revert any of the files in the working tree. I don't have a problem with occasional commit that changes more than "one" thing, provided that this is clearly stated in the commit message and that the changes are small in overall. I saw some commits on other projects where the commit message relates only to the 30% of the actual change, and the rest of the code is WTF. Quote from "Wub" Curious to know what workflow Zalewa has. If I discover that that an unpushed commit still needs some work, I don't shy from amending or 'hg histediting' it. I know that Hg principle is to always keep the "real" history of your work, but I prefer to not keep garbage in the repo. Torr applies the same principle to the pull requests for Zandronum and my office also uses the same principle for all software. It works. All changes that do not break the master/default, should be on the master/default, even if work is partial. Of course, if master/default software needs to be broken to do the change, then branching is reasonable. Moreover, if we ever need a "stable" or "patch" branch, we should branch off from master/default in a sensible place and then cherry-pick/graft commits from master onto it. We also use the same principles at the office where there are many developers working on the same repo. Stable/patch branches are normally not merged back into master. It works. Quote from "Wub" Curious to know what tools Zalewa uses. For git, the command line is the only user interface that is user friendly. You just need to Google 'git lg1 lg2' and add it to your ~/.gitconfig and set mergetool to kdiff3. The rest works correctly out of the box. For Hg, I didn't bother to learn the command line, but whenever I have to use it, I find it horribly clumsy and uncooperative with other GNU tools. It's amazing they actually got 'hg histedit' to work in the same way as 'git rebase -i'. One would expect that they would screw it up too. TortoiseHg, in this case, is the best UI. Quote from "Wub" Adopt a 50/72 [message] style If anything, I'd go with 80/80. TortoiseHg still doesn't allow to wrap lines in commit message viewer, so if you go overboard, you'll get a horizontal scroller, which is annoying. Manual formatting of the lines to correct lengths prevents that. When I occasionally need to use 'hg log', the shell or cmd.exe will wrap long lines for me. As for the current format of the commit message: I have already proposed to make the same change years ago, but at the moment Blzut3 stated that he wants to keep the current format because its easier to create a changelog from it. We have eventually concluded that changelog != commit messages. Changelog entries like "- typo fixed" or "- some code change irrelevant to the end-user" or "- fix stuff that was broken in previous commit" are not helpful to the end-user at all. Personally, in all new projects I go with the "best practice" standards recommendation of subject/body separation and use self-imposed limit of 80/80. For Doomseeker, I came to accept that the current format is the accepted format and I also accept the occasional horizontal scroll bar that happens because of it. In any way, I have no strong feelings one way or the other on keeping the current format or changing it.

(0018716) Zalewa (developer) 2017-11-01 18:05	AOSP, could you comment why you oppose?

(0018718) WubTheCaptain (reporter) 2017-11-02 01:32 edited on: 2017-11-02 01:35	Thanks for the detailed explanation, Zalewa. I agree with most of it, though I have few comments on things I disagree. Quote from Zalewa Stable/patch branches are normally not merged back into master. In gitworkflows(7), this is the opposite: Maintenance patches (e.g. 1.1.1) are committed to maint branch, then merged to master (e.g. 1.2). In this workflow, commits are only cherry-picked if they were forgotten. The reason? Both maint and master can reference to the same commit by same hash then, so a cherry picked commit is not under two hashes in the repository. See The gitworkflows(7) illustrated by K. Tateishi for a detailed illustration. Quote from Zalewa If anything, I'd go with 80/80. There's 13 characters of left-padding in hg log (command line interface, I know). I know the terminal emulator wrapping, but like to stay loyal to 80x24 terminal size. hg -v log doesn't have the padding at all. 50/78 or 50/80 sounds reasonable to me. Consider the proposal of 50 for title/summary to be a suggested soft maximum: If it needs to be longer, it will be longer (at hard max of 80 characters). 'https://medium.com/@preslavrachev/what-s-with-the-50-72-rule-8a906f61f09c [^]' Quote from Zalewa Changelog entries like "- typo fixed" or "- some code change irrelevant to the end-user" or "- fix stuff that was broken in previous commit" are not helpful to the end-user at all. Agreed, but for the developer it stills leaves something to desire to explain why the changes were made and locating those changes quickly. A changelog is a seperate summary, hand-crafted from commit messages. (Except if it's a GNU project. Then ChangeLog would be the commit log dump, and NEWS news would be a summary.)

(0018719) Zalewa (developer) 2017-11-02 10:35	I have viewed this presentation and while it does sound reasonable and I understand what kind of problems it allows to fix, I have doubts if we have these problems in the first place. We don't do topic branches because most features can be implemented in a single commit. Server browser is not rocket science. I see no benefit in keeping a railway in the repo history instead of a monorail. If feature is huge, like IRC client, it can be argued that an iterative implementation of this feature is already useful to the end-user. So, the first commit with a primitive implementation can already serve as usable (though, naturally, useless) IRC client. It can be argued that such implementations can be done on separate branches to avoid decreasing the overall quality of the software, but in the long run we don't release that often and once release happens, it happens in a time when we know there's no unfinished work. There's usually no rush to produce a release, therefore there's rarely unfinished work that prevents releasing. Doomseeker does not grow rapidly (as you probably have already noticed). We work on weekends or during holidays or vacation. Since we control all code and we keep it in a single repo (thank Crom), there's been little incentive to maintain absolute stability of API/ABI. We still prefer to maintain it, but when its stability gets in the way of resolving an issue cleanly then we prefer to break it. Usually serious bugs are revealed right after the release, which means that bugfix releases, if any, happen usually after the feature release. There isn't much new code accumulated. We don't have problems with sneaking a new feature into a bugfix release. I also wonder who starts working on a new repo right from many topic branches, when an application must grow on a common basis - single build system, single database access layer, single API layer, single UI framework - this stuff must be resolved before anyone can think of building independent features. And even then, during the initial development, there's still lots of flux that will affect the common root of all features.

(0018720) Zalewa (developer) 2017-11-02 10:37	This ticket itself breaks the rule it preaches about because commit message formatting and commit contents are two separate things.

(0018723) WubTheCaptain (reporter) 2017-11-02 13:39	Quote from gitworkflows(7) Many ideas apply in general, though the full workflow is rarely required for smaller projects with fewer people involved. In a small (git-hosted) project, master and maint have been enough for me. Quote from Zalewa We don't do topic branches because most features can be implemented in a single commit. I've kept topic branches local to my computer, semi-throw away. Quote from Zalewa This ticket itself breaks the rule it preaches about because commit message formatting and commit contents are two separate things. I'm aware, it seemed easier to have a central discussion though. Mentioning gitworkflows(7) also was not the intent. I always regret these things later. There's no strong signals in favor of a change as of now, so I'll close this ticket (though I'd still personally like to see a change). I also now better understand why things are currently done the way they are, and there's now a public record of them. (For the lack of a better communication channel for Doomseeker.) Thank you everyone.

This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why.
Supporters:	No one explicitly supports this issue yet.
Opponents:	DrinkyBird

Issue History
Date Modified	Username	Field	Change
2017-10-30 02:17	WubTheCaptain	New Issue
2017-10-30 02:17	WubTheCaptain	Severity	minor => text
2017-10-30 02:19	WubTheCaptain	Description Updated	View Revisions
2017-10-30 02:37	WubTheCaptain	Steps to Reproduce Updated	View Revisions
2017-10-30 02:40	WubTheCaptain	Steps to Reproduce Updated	View Revisions
2017-10-30 02:43	WubTheCaptain	Steps to Reproduce Updated	View Revisions
2017-10-30 03:03	WubTheCaptain	Steps to Reproduce Updated	View Revisions
2017-10-30 04:28	Blzut3	Note Added: 0018678
2017-10-30 04:43	WubTheCaptain	Assigned To	=> Zalewa
2017-10-30 04:43	WubTheCaptain	Status	new => feedback
2017-10-30 04:59	WubTheCaptain	Note Added: 0018680
2017-10-30 04:59	WubTheCaptain	Status	feedback => assigned
2017-10-30 04:59	WubTheCaptain	Status	assigned => feedback
2017-10-30 05:07	WubTheCaptain	Assigned To	Zalewa =>
2017-10-30 05:07	WubTheCaptain	Status	feedback => new
2017-10-30 05:07	WubTheCaptain	File Added: mercurial-merge-flow.txt
2017-10-30 05:09	WubTheCaptain	Note Added: 0018681
2017-10-30 05:13	WubTheCaptain	Status	new => feedback
2017-10-30 05:43	WubTheCaptain	Note Added: 0018682
2017-10-30 05:43	WubTheCaptain	Status	feedback => new
2017-10-30 05:52	WubTheCaptain	Note Edited: 0018682	View Revisions
2017-11-01 15:09	Zalewa	Note Added: 0018714
2017-11-01 18:05	Zalewa	Note Added: 0018716
2017-11-02 01:08	WubTheCaptain	Status	new => acknowledged
2017-11-02 01:32	WubTheCaptain	Note Added: 0018718
2017-11-02 01:32	WubTheCaptain	Note Edited: 0018718	View Revisions
2017-11-02 01:35	WubTheCaptain	Note Edited: 0018718	View Revisions
2017-11-02 10:35	Zalewa	Note Added: 0018719
2017-11-02 10:37	Zalewa	Note Added: 0018720
2017-11-02 13:39	WubTheCaptain	Note Added: 0018723
2017-11-02 13:39	WubTheCaptain	Status	acknowledged => closed
2017-11-02 13:39	WubTheCaptain	Resolution	open => denied

Relationships