CI system maintainability
Friedrich W. H. Kossebau
kossebau at kde.org
Thu Mar 28 14:39:54 GMT 2019
Am Donnerstag, 28. März 2019, 11:27:44 CET schrieb Daniel Vrátil:
> I'm completely fine with mandatory code review for everything and I'd be
> happy to have this in PIM. I think the biggest problem in PIM to overcome
> will be finding the reviewers - I dare say I'm currently the only one who
> has at least a little idea about what's going on in Akonadi, so getting for
> instance Laurent to review my Akonadi patches might be hard - same for me
> reviewing Laurent's KMail patches. This will require non-trivial amount of
> effort for all members of the community to gain deeper understanding of the
> other components within the project and a willingness to step up and do a
> code review even if they don't feel 100% comfortable with the code base.
> But I believe that in the long run the benefits clearly out-weight the
So why do you not do this already? Why would you only do this is there is a
policy requiring you to do it?
And why do you think this policy-driven behavioural change will work or is
needed with everyone else?
Also, how do you ensure the review is actually of quality?
And not just socially driven "+1 for my buddie!", where buddy then also
mentally shifts part of responsibility to other buddy, because, he, more eyes
means I do not need to do the full work myself, glitches will be caught be
them surely! Reviewer found a code formatting issue, done here, review
The opposite also has been seen, reviewers feel they need to do "real" review
and find things, so start to nitpick or to demand wrong changes, wasting
For myself I know I will happily have other people review my patches, but
only if there are qualified people to be expected to do it with the needed
quality in a reasonable time.
Then, trying to force by that policy other people to become proper specialist
for certain other code projects to do qualified reviews, actually is a trade-
off. They will have less time for their actual code project, becoming less a
specialist there (or having time to review other people's contribution to
We need more contributors, not force existing contributors to distribute
their abilities & resources over more projects (and thus having less for
their actual one).
I am also not aware of many contributors to KDE projects who are not capable
to make a responsible decision between the few obvious simple fixes and those
normal changes which better get feedback from others first. If one is unsure
about one's post-beer late-night quick hack, one will upload it for review,
no? At least if one's previous late-night commits turned out to be late-night
mental state impacted.
To deal with those few which seem challenged to do that decision properly, I
find such a policy for everyone harmful, I know it would impact my
contribution willingness, when I come across a typo or other simple and
obvious to fix things. And just leave the garbage around along my current
Besides, it will not solve the issue this thread is about, with some people
not caring (quickly) enough if CI builds fail or if there are regressions in
Review builds once implemented and deployed will help there partially as a
side-effect only, catching some build fails before. But also not if this
breaks (e.g. due to API/behavioural changes) depending projects, as CI at
least currently does not rebuild dependent projects (cmp. also T7374).
A society with people doing things only due to policies and not intrinsic
motivation seems very flawed to me, makes me wonder why people are in there
given no-one forced them into this society.
Commit_Policy#Code_review_by_other_developers has some policies already,
which should be enough:
1.1 Think Twice before Committing
1.2 Never commit code that doesn't compile
1.3 Test your changes before committing
1.4 Double check what you commit
1.10 Code review by other developers
1.11 Take responsibility for your commits
1.12 Don't commit code you don't understand
Well, perhaps could be amended by an explicit note about keeping CI working.
Enforcing review of any commits IMHO should be only done for people who
notoriously failed to comply with those existing rules. If we are unable to
pinpoint those people, talk with them and make them comply or sort out their
reasons to not yet comply, but instead create as workaround new generic
policies for everyone, we make this a worse society. And also a less
effective, with more rubber stamps needed.
More information about the kde-core-devel