cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Laszlo Hornyak <laszlo.horn...@gmail.com>
Subject Re: Code quality, QA, etc
Date Fri, 07 Feb 2014 23:40:00 GMT
Hi,

I used to work with gerrit in OS projects and I think the tool is great,
the integration with jenkins is cool.
One problem could be when jenkins infrastructure problems are frequent and
developers start to ignore warnings from jenkins.
With my particular project we were also frequently hit by gerrit outages. I
do not know the reason since I did not operate the infrastructure, but
having 1-2 outages per week was normal.

This is the technical part and I am sure you can make a more reliable
service.

We also had Review-then-commit process, and in general I had a bad
experience with the process. I do believe the code review is necessary in
an open source project and it can improve quality, but at the same time the
costs (in time and lost braincells) are very high and the existence of a
process does not guarantee that the quality will improve. No process
replaced thinking so far.
Once I complained about having the 30th version of a patch that in my
opinion was quite simple and then someone answered that he is already over
the 40th review. It took several months to push something through the
process. And those numbers just kept growing. We collected some of the top
reasons with my team:
- the review was not really a review, the reviewer only looked at the code
in firefox. Never checked out, never ran the tests.
- reviewer expectations were various even in the same language and module
between reviewers, unfortunately this was not documented, so you had to use
the try-and-fail process to learn individual reviewer preferences, it took
quite a lot of time since the team was huge
- one had to wait for review sometimes for several weeks. Meanwhile the
patch got outdated and had to be rewritten, and then the whole process
started over again.
- Also, reviewers blocked at the first issue found in the patch. This was
usually in the commit comment, they did not like it. So you change the
commit comment and hope that next time the guy will read some actual code.
Maybe he will block on something like he does not like your variable name.
This is especially annoying when you send an urgent fix.
- The typical reason for merging a patch was the release deadline. Just a
few days before the deadline they merged everything. So we have spent
several months and still only the developer tested the code.

In my opinion a review tool is not enough to make the review process
productive, you need good reviewers.

Regards,
Laszlo

On Fri, Feb 7, 2014 at 4:50 AM, David Nalley <david@gnsa.us> wrote:

> Hi folks,
>
> We continue to break things large and small in the codebase, and after
> a number of different conversations; I thought I'd bring that
> discussion here.
>
> First - coding quality is only one factor that the PMC considers when
> making someone a committer.
>
> Second - CloudStack is a huge codebase; has a ton of inter-related
> pieces, and unintended consequences are easy.
>
> We also have an pretty heady commit velocity - 20+ commits today alone.
>
> Some communities have Review-then-commit - which would slow us down,
> and presumably help us increase quality. However, I am not personally
> convinced that it will do so measurably because even the most
> experienced CloudStack developers occasionally break a build or worse.
>
> We could have an automated pipeline that verifies a number of
> different tests pass - before a patch/commit makes it into a mainline
> branch. That is difficult with our current tooling; but perhaps
> something worth considering.
>
> At FOSDEM, Hugo and I were discussing his experiences with Gerrit and
> OpenDaylight, and he thinks thats a viable option. I think it would
> certainly be a step in the right direction.
>
> Separately, Jake Farrell and I were discussing our git-related
> proposal for ApacheCon, and broached the subject of Gerrit. Jake is
> the current person bearing most of the load for git at the ASF, and
> he's also run Gerrit in other contexts. He points out a number of
> difficulties. (And I'd love for him to weigh in on this conversation,
> hence the CC) He wants to expand RB significantly, including
> pre-commit testing.
>
> So - thoughts, comments, flames? How do we improve code quality, stop
> needless breakage? Much of this is going to be cultural I think, and I
> personally think we struggle with that. Many folks have voiced an
> opinion about stopping continued commits when the build is broken; but
> we haven't been able to do that.
>
> --David
>



-- 

EOF

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message