Rant regarding code review issues

Wilfred Smith wilfredsmith at fb.com
Sat Sep 14 04:52:07 AEST 2019


I believe this is a justified rant. This is not intended to offend, but to assert that the process is broken.

I strongly assert that there should be a formal list of commit rules. If the rules were actually documented and published, one could go through the checklist to ensure that the commit conforms. The result would be less time wasted by the committer and the reviewers, as well as more consistency.

When I go tooling down US-101 at 90 mph in the carpool lane, texting my illegal alien girlfriend as I’m driving to the convenience store, in a stolen red convertible, with a bottle of fine scotch in one hand and an unregistered AK in the other, at least I know which rules I’m violating or likely to be violating when I hear helicopters overhead. 

But it sucks to have a commit booted for reasons that (a) I could not reasonably be expected to have known previously (because I read the documentation, of course) and (b) I may legitimately disagree with (e.g. the insanity that anyone holds to an 80-column rule instead of 132 in a day of 4K monitors and 75-character class names inside nested namespaces that are just as long, not counting fully decorated traits). Oh…right…just use “auto”.

The process should be somewhat predictable, preferably programmatic.

Mind you, I have no problem with complying with the maintainer’s rules, but I’d like to have some chance at getting a trivial commit passed on the first shot. Two lines of BitBake config, that actually accomplish what is intended, fetches a half-dozen complaints?

It helps the maintainer if committers are able to self-police 98% of the issues, and makes the entire process seem less hostile.

Also, there are too many places to put the same information. I quoted the warning messages I was remedying in the patch set comments, but summarized in the commit message because that seemed like the right thing to do. And that got me another downvote.

Now forgive me for my rant. I gotta pick up some M&M’s for my girlfriend before 5th period or she’ll be very cross with me.




More information about the openbmc mailing list