Mailinglist Archive: opensuse-factory (1029 mails)

< Previous Next >
Re: [opensuse-factory] O Factory - Where art Thou?
Am 29.11.2013 11:12, schrieb Stephan Kulow:
On 29.11.2013 10:24, Josef Reidinger wrote:

Tricky part is that for manual code review you need time and with
increased number of packages it is not possible unless manpower is
increased. I agree with you that code review is really important and
for example in Yast we see really good impact on code quality, but
experience is that for product quality automatic testing is more
important.

We thought about that too and it's indeed a limiting factor, but IMO
having reviews is so valuable that I wouldn't want to throw it away just
because it's hard.

Right now reviews have no priority at all, so the more packages we have
the easier it is to get lost.

I agree. It's also quite a task to keep up with recent development and decisions.

And there is no support for sharing work
in reviews the webui interface to do reviews is suboptimal too (it's
dominated by the diff and the discussion is very much hidden).

The request view could hava (ajax-powered) tabs, where the first one only shows the discussion, the most important facts (like the review header) and the history (in a meaningful way). I would also put the build results and rpmlint issues on the first tab.

Second tab would be the diff in all it's beauty. When build results and rpmlint issues moved to the first tab, the diff can take up the full width and people need to scroll less.

Form controls (accept/decline/add-review) would have to be shown as a seperate bento box (OBS code insider) down below as it is today.

Github users will find this vaguely familiar.

On the other hand we do have enough experienced packagers who can do
reviews, but at the moment being a reviewer or not is black & white.
So if you volunteer to review because you know how to package perl
packages well, you end up being part of the same team as the one
reviewing kernel submissions. So possibly we have do have the reviews
done by more specific groups instead of one big group?

So far, the review team has been a bunch of generalists of which most have a rather heavy footprint in the distro. This helped to keep up with a general level of quality. However, quality reviews take a considerable amount of time. Time which most of us volunteers don't always can afford.

Dedicated topic groups are a neat idea, but I consider this the (sole) responsibility of the devel project. Instead of just accepting all random <strike>crap</strike>hot stuff into their devel project, maintainers should discuss and weigh the pros and cons. This is happen to different degrees in different projects. However, hoping that the review team will just catch everything is definitely the wrong mood. But my estimation is that ~75% devel project maintainer do it that way (maybe not knowingly).

So rather than "fixing" the review team, we should fix the devel projects maintainer mindset. Rob and I started this a while ago under the "maintainer model cleanup" moniker. Ultimately (and due to their lack of special knowledge), the review team members have to trust the devel project maintainers that they do the right thing. Reviewers can only catch general issues or stuff that went in unnoticed.

So every time I do reviews, I try to check opensuse-factory/-packaging for news (like tirp/krb5 ATM). But even though I try to do as good as I can, I won't dig thru all the new code, all upstream bug-trackers and ML discussions. I simply trust the devel project maintainer that this happened already. Otherwise, I would have to spend ~1 month per Kernel submit request ;-)

That's why reviews emphasize on the packaging side of things. That's really what upstream devs pay little attention. Therefore we have countless policies that distill years of experience, middle-grounds and common agreement. Of course they aren't perfect and sometimes outdated. But this is an important part of the "quality" of our distro. Otherwise we would just ship upstream tarballs.

That would leave us close to the Signed-Off tags kernel patches bear,
but the OBS has no support for marking patches as reviewed by $Josef.
Yet another idea how to improve things.

This partly works today. Everybody is free to leave his comment on each Factory submit request. But I agree, the integration and tooling has room for improvement :-)

--
To unsubscribe, e-mail: opensuse-factory+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: opensuse-factory+owner@xxxxxxxxxxxx

< Previous Next >