CI system maintainability

[Dr.-Ing. Christoph Cullmann]

Hi,

> Hi,
> 
> On Thu, 28 Mar 2019 at 15:21, Kevin Ottens <ervin at kde.org> wrote:
> 
>> Hello,
>>
>> On Thursday, 28 March 2019 14:33:59 CET laurent Montel wrote:
>> > I am against to force mandatory review, as it will create a lot of lose
>> of
>> > time,
>>
>> As I said, unpopular.
>>
> 
> I don't get why mandatory code reviews are so unpopular.
> 
> I don't care if you lose time. I don't want the guys building my house to
> cut corners mixing my concrete because it's going to save time. Why are you
> in such a massive hurry to make changes to software which for example holds
> access to my Google Account password? In fact, the very fact that you make
> this argument makes me wonder if I'm running trustable code on my computer
> at all, because apparently doing it quickly is far more important than
> doing it right.
> 
> As a user, I simply do not want unreviewed crap running on my computer.
> Yes, crap, because no software engineer writes bug-free code all the time,
> and if you're so overconfident that you don't need reviews on even your
> one-liners, you're probably too overconfident to be writing good code
> anyway, so I'm going to operate on the presumption that if the code hasn't
> had more than one pair of eyeballs ever looking at it, it's crap.
> 
> As a developer, I know that even one-liners, and especially one-liners, the
> sort where you think "meh, this is a tiny little thing, I don't have to be
> careful" are the ones that have the most dangerous typos and unintended
> bugs. Reviews catch that.
> 
> In a project like PIM, if the code hasn't been through review, which
> independent party do I trust to verify that you're not, for example,
> leaking my Google password to some world-readable tempfile? Do you really
> expect every user to read the entire codebase for themselves and make sure
> that's not being done? The whole point of having all the code out in the
> open for independent audit purposes, to protect your security and privacy
> and what not is completely moot if no one else actually looks at the code
> anyway. And let's be honest, the code quality of some of KDE's projects - I
> wouldn't touch them with a six-foot pole. The ones I would touch though,
> all have multiple people looking at the code and reviewing everything that
> goes in.
> 
> Let me be very clear - even if you're the best damn programmer on the
> planet, if *you* wrote the code, I do not trust *you* one inch to tell me
> that that code is correct. That verification needs to come from someone
> else, someone who does not have a conflict of interest in seeing that code
> get into production. This is nothing personal, this is confirmation bias on
> the author's part which leads to issues that even though they might be
> infrequent, usually have catastrophic impact.
> 
> And if "culture" trumps over engineering best practices, it follows that I
> should just stop using software produced by this entity because who knows
> what it's doing.

Mandatory code reviews are nice, if you have the manpower.

Look at our phabricator, look for example at the queue of reviews for KTextEditor.

The team is small and the code is complex and old (not rocket-science, it is a text editor,
but still...).

I and others tried to get more reviews done in the past, but actually I merged more
than once stuff that I reviewed but it did break the CI.

In most cases I fixed it myself afterwards or reverted again, but a few times
I needed to get ping'd by others that I was stupid, too.

In the current case discussed an error happend, it could have happend exactly the same
way if for example "me" would have reviewed it.

A lot of the changes which are at the moment in the queue are stuck because

a) lack of time to review the change
b) lack of time to ever be able to test the change

For any non-trivial behavior change (especially for features you not use yourself at all),
any meaningful review is a full-time job. e.g. in our company you would let some student
test the changed behavior some days. This is just not feasible for me, and yes,
for some of these changes, rather than abandoning them (and trashing precious work others did),
I will somewhen apply them with close to zero real testing.

Greetings
Christoph

-- 
----------------------------- Dr.-Ing. Christoph Cullmann ---------
AbsInt Angewandte Informatik GmbH      Email: cullmann at AbsInt.com
Science Park 1                         Tel:   +49-681-38360-22
66123 Saarbrücken                      Fax:   +49-681-38360-20
GERMANY                                WWW:   http://www.AbsInt.com
--------------------------------------------------------------------
Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234