Changes to our Git infrastructure

Ian Wadham iandw.au at gmail.com
Tue Jan 6 06:40:01 GMT 2015


First of all, Thomas, I forgive you (and others) for nit-picking, etc. but please
don't do it again.

And I thank you, Thomas, for two positive suggestions re fixing the Dr Konqi
reporting failure, both before 10 October and during its resurgence, in a new
guise, in November and December (see thread "Dr Konqi still misbehaving -
advice needed", on this list).

Also, I am grateful that you took the time to TEST what was going on, in Linux,
in December --- something I am unable to do --- and to nail down the cause of
the resurgence.

On 06/01/2015, at 11:47 AM, Thomas Lübking wrote:
> On Montag, 5. Januar 2015 23:53:02 CEST, Boudewijn Rempt wrote:
>> I'm just trying to make clear that reviewboard is a crappy tool inciting people to write crappy reviews that drive people away. 
> 
> And I was trying to make clear that those "crappy reviews" are just the house cleaning stuff that should ideally go along w/ the functional reviews.
> That you believe it's the only kind of review you get implies that you would simply not have gotten any other review otherwise. Not ideal, but rather unrelated.
> 
>> Apart from any other nonsense about cultural differences (the standard cop-out from Dutchmen and Germans -- I ain't rude, I'm just honest, it's cultural!)
> 
> Sorry, I was just trying to understand what could get anyone scared about a very simple and direct "please fix this, this and that" list - it would have to be fixed anyway at some point.

Not scared, just bloody annoyed.  Also, with Dr Konqi, I was trying to kick a
goal, but it seemed that members of my own team kept tackling me… :-)

> I'm sorry if you or anyone feels offended by the way it's presented, but to change that, one would first need to have a remote idea, what's so driving away about it. And sorry again, but I really cannot even imagine - wheter you may call that rude or dumb.
> 
> 
> 
> Ian and the DrKonqui bug:
> ---------------------------
>> I think that people should read Ian's mail, with attention:
> I have read Ian's mail and I do recall his review requests on DrKonqi.
> 
> I agree that the process was anything but fluid - though ultimately successful.
> 
> Apparently nobody actually felt in charge of DrKonqi then (and now, since Ian will apparently be sadly lost, again), so other developers, unfamiliar w/ DrKonqi commented on the patches - the first two of them actually focussing on OSX specific (or rather triggered) issues.

So, there are middle --- and professional --- ways for a reviewer to handle this.

   a) "I do not know anything about Dr K, but I will try and find someone who does."
   b) "Unfortunately there is nobody available any more who knows anything about
        Dr K, but I (or another suggested guy) will try to help.  How about we take this
        offline via email or IRC and then you can walk us through the problem you are
        trying to fix, its significance and impact and how you are going about fixing it…"

The polishing (fixing nitpicks, etc.) should come *after* the stone is cut.

Going straight to that mode is inappropriate because it conveys the message,
"The problem you are trying to fix is unimportant to us".  Metaphors about Nero
and violins spring to mind as the reviewee grinds his teeth and gets the polishing
tasks out of the way, one by one…

> The last one addressed DrKonqi being broken due to bugzilla changes.

My beefs with ReviewBoard are not confined to the Dr K episode.  I have several
other examples where I wanted to fix code I do not regularly maintain and someone
suggested (or I volunteered) to post the changes to ReviewBoard.  If there is no
maintainer, as is often the case these days, you are likely to get either silence or
nitpick, etc.

No amount of new technology, neither Gerritt nor the energy of cats confined in a
bag, can help.  "There are management solutions to technical problems, but there
are no technical solutions to management problems", as a colleague of mine used
to say.

> It was presented first on 30/9/2014, saw an immediate coding style and function design review, resolved 3/10/2014.
> Followed up by a minor nitpick to not use fprintf, but kdebug and a call for someone w/ experience on the codebase to review on 5/10/2014
> 
> On 6/10/2014 Ian worried that he's perhaps "the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now" and sugested to push the patch within the next 24h.
> 
> This raised immediate concerns from the release team about the size of the patch, discouraging to submit it to 4.14 what caused a hyperactive 7/10/2014 with the discussion leading to the consensus that a vastly simplified version of the patch would still be the best option for 4.14 (and suggestions on how to simplify things) with the result being finally presented 9/10/2014 and submitted and submitted "in time" 23h later.

Yeah.  This one-minute-to-midnight stuff is all very exciting to younger blood,
but it is NOT the professional way to do things.  If there had been a day to spare,
someone could have tested the Dr K change on Linux and avoided the resurgence
of the problem.  As it was, it had to be tested "in the field", see:
https://bugs.kde.org/show_bug.cgi?id=337742#c30, also #c22 to #c62

BTW, re the "Dr Konqi still misbehaving - advice needed", the latest fix has not been
tested on Linux, in spite of repeated requests by me.

HERE WE GO AGAIN, CHAPS…  Applications/14.12.1 GETS TAGGED IN 2 DAYS… :-(

> I would fully agree that 7/10/2014 should have been 30/9/2014, but the most important thing is:
> 
>  IT TOOK A VETO FROM THE RELEASE TEAM            and thus
>  THE THREAT OF A BROKEN DR KONQI FOR KDE SC4
> 
> to cause major interest in the patch, what is, given the DrKonqi bug was reported
> long before, a clear sign that nobody actually had a special interest.
> In the end, blind ppl. were supposed to guide the one-eyed.

Heh, heh!  http://en.wikipedia.org/wiki/The_Country_of_the_Blind

I often think of that story as I work on KDE software.  The "culture" of the
valley overrides any previous experience or abilities Nuñez possesses.

>    AND NO TOOL ON EARTH WILL EVER FIX SUCH A SITUATION.
> 
> Tools are as good or bad as the ppl. that use them.

Exactly.

> The problem is *not* that RB encourages nitpicks,

Exactly, except that nitpicks, etc. turn off people who would like to help.

> but the problem is that there's completely unmaintained code where
> nobody feels qualified to sign off patches.

Exactly.  And there are simple, technology-free solutions to that problem,
if anybody is interested.

> Ian is certainly the one person permitted to retire whenever he feels like,
> but /actually/, now that the worst time is passed, he would be the one to fix
> *this* problem for DrKonqi, ie. become the person to sign off patches.

No way.  I only ever worked on KCrash, Dr Konqi, kdeinit4, klauncher,
kded4 and friends because these things did not work as intended on
Apple OS X.  I want to move on to other things on that platform now.

Some other problems with ReviewBoard, which might be just the same
in any other reviewing tool the KDE Community may adopt.

  a) There is no encouragement for the reviewer to build and TEST the
       patch, independently of the reviewee.
  b) Although the display facilities of code in RB are very good, there
       is a temptation to focus on the "colored bits", and in a purely textual
       way, as opposed to reviewing the change in the whole context of how
       the software is going to operate.
  c) Patching encourages incremental, "evolutionary" development,
       rather than standing back and taking a "design" view of the way
       things are developing.  BTW, ReviewBoard supports post-commit
       reviews of several patches or commits together, but that feature
       is turned off in the KDE usage [1].
  d) ReviewBoard does not support design or specification reviews.
  e) ReviewBoard does not support "walkthrough" reviews of entire
       bodies of code.  I would say Dr Konqi is long overdue for one
       of those.  Maybe also kinit and friends (in KF5).

Before anyone starts talking about "cathedrals and bazaars", re
c), d) and e) above, please consider that a little bit of "cathedral"
never hurt anyone.

Indeed, the really great World Heritage bazaars in Tabriz, Shiraz,
Esfahan and Istanbul were each designed by somebody.  They did not
just happen by a few stallholders getting together.  And the classic
Islamic cities are designed for the mosque and the bazaar to be
close together, so that merchants and customers can easily go
to prayers several times a day.

All the best, Ian W.

[1]
Re c) above, I have read the documentation and pros and cons in:
https://www.reviewboard.org/docs/manual/2.0/users/getting-started/what-is-code-review/



More information about the kde-core-devel mailing list