Rant about unprofessional workflow [LWN.net]

Rant about unprofessional workflow

Posted Nov 7, 2016 12:19 UTC (Mon) by mathstuf (subscriber, #69389)
In reply to: Rant about unprofessional workflow by NAR
Parent article: A discussion on stable kernel workflow issues

Every substantive change, sure, maybe a mention in an issue tracker is important, but for issues like malloc/delete mismatches, whitespace fixes, doc typo fixes, and the like, it is just extra work. That's why "every commit requires an issue" is block headed. Maybe every topic/merge request, but certainly not every commit.

to post comments

Rant about unprofessional workflow

Posted Nov 7, 2016 14:34 UTC (Mon) by excors (subscriber, #95769) [Link] (1 responses)

If you require every commit to reference an issue, then for trivial commits it might take a couple of minutes to copy a few lines from the commit message into the issue tracker which nobody will ever look at again, so it's wasted time. (For multiple closely-related commits you could combine them into a single issue, like "Fix coding style in xyzzy driver", so it's really an overhead per group of related work, not per commit.)

But if you don't require every commit to reference an issue (and enforce that requirement), someone will occasionally make an important substantive commit without an issue, which will turn out to introduce some bugs or need backporting or need some documentation written about its rationale etc, and someone will have to spend hours searching through mailing lists and LWN articles trying to find all the possibly-years-old discussions about that commit (earlier versions, code reviews, related bug reports, etc). That's also wasted time, which could have been saved if the discussion about the commit had been tracked in a single place (even just as a collection of links to mailing list threads).

If you require only substantive commits to reference an issue, you'll waste time deciding and then arguing with people over whether a commit is really substantive (e.g. a malloc/free bug could be a serious security vulnerability so surely that counts?), and you'll still sometimes fail to track a commit that should have been tracked.

I think it's unclear which approach wastes the most time overall, but the most predictable is to require an issue for every commit - it's a mildly irritating overhead when writing patches, but it's easy to get into the habit of using the issue tracker for everything, and it can save some rare but substantial headaches in the future.

Rant about unprofessional workflow

Posted Nov 7, 2016 18:03 UTC (Mon) by bronson (subscriber, #4806) [Link]

Furthermore, if you're iterating so fast that creating a tracker entry is an unbearable overhead... I'm not sure your work belongs in the stable branch.

Rant about unprofessional workflow

Posted Nov 7, 2016 16:31 UTC (Mon) by NAR (subscriber, #1313) [Link] (2 responses)

Whitespace, typo fixes, merge errors - they don't warrant issue tracker entries. The malloc/delete mismatches could be real, user facing errors (e.g. destructor not called), these should be tracked.

Rant about unprofessional workflow

Posted Nov 7, 2016 17:27 UTC (Mon) by madscientist (subscriber, #16861) [Link] (1 responses)

Merge errors should be tagged with the tracker entry which introduced the merge error, so that an attempt to cherry-pick the broken commit has a chance of noticing the commit that fixes the error as well.

For whitespace cleanups and typos, you can have a catch-all tracker entries (as mentioned above) for "whitespace cleanup" and "comment typos" that is just used for all those types of issues. It incentivizes people to break these out into separate commits anyway, which is a good thing.

When I introduced a similar requirement locally (using Git hooks on the server to reject pushes where the comment didn't provide the proper information) I originally thought it would be good to have an "out" where you could use a special tag like "trivial" or something and avoid creating a tracker entry. Others in the team were against this and wanted to require a real tracker entry for every commit so that's what we did, and it has been fine.

Rant about unprofessional workflow

Posted Nov 10, 2016 18:02 UTC (Thu) by Wol (subscriber, #4433) [Link]

The trouble is, (a) managing the kernel is like herding cats, and (b) many of them are blind :-) Without a spec it's too easy to make a mistake, and nobody wants to specify or document what *should* happen, so all too often it doesn't ...

Cheers,
Wol