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
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
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(a)opensuse.org
To contact the owner, e-mail: opensuse-factory+owner(a)opensuse.org