8:25 a.m.
On 8. 4. 2021, at 7:18, Yedidyah Bar David <didi(a)redhat.com>
wrote:
On Wed, Apr 7, 2021 at 6:45 PM Michal Skrivanek
<michal.skrivanek(a)redhat.com <mailto:michal.skrivanek@redhat.com>> wrote:
>
>
>
>> On 7. 4. 2021, at 12:04, Yedidyah Bar David <didi(a)redhat.com> wrote:
>>
>> On Tue, Aug 11, 2020 at 11:05 AM Tal Nisan <tnisan(a)redhat.com> wrote:
>>>
>>> Hi everyone,
>>> As you probably know we are now in a mode in which we develop our next
zstream version on the master branch as opposed to how we worked before where the master
version was dedicated for the next major version. This makes the rapid changes in master
to be delivered to customers in a much higher cadence thus affecting stability.
>>> Due to that we think it's best that from now on merges in the master
branch will be done only by stable branch maintainers after inspecting those closely.
>>>
>>> What you need to do in order to get your patch merged:
>>> - Have it pass Jenkins
>>> - Have it get code review +2
>>> - Have it mark verified +1
>>> - It's always encourage to have it tested by OST, for bigger changes
it's a must
>>>
>>> Once you have all those covered, please add me as a reviewer and I'll
examine it and merge if everything seems right, if I haven't done it in a timely
manner feel free to ping me.
>>
>> Is the above still the current policy?
>
> Hi Didi,
> well, yes it is. what’s the concern?
No "concern", other than I noticed a few times that people other than
Tal merged patches, and yesterday I did the same [1] after getting +1
that’s master branch, not stable branch.
from Sandro and consulting him, seeing that I have permissions. IIRC
I
didn't have permissions until recently, so I wondered if anything
changed.
If the re-granting of permissions was a mistake, let's revert. If it's
on purpose, perhaps clarify the situation.
we did a major cleanup of stale people and the convoluted system of groups that
accumulated in the past
we now just use a simple “regular” and stable maintainers list. +2 are for respective
areas, we do not have a per-area granularity in ovirt-engine project, it’s really just an
agreement that you shouldn’t merge patches in areas that are not yours.
oh, no w I get it, you really talk about submitting, not +2:) ok, yes, so that has
changed. we’ve concluded that we can go back to previous mode of not distinguishing
between +2 and merge rights.
everything else above applies about the patch quality criteria, it’s just that indeed
anyone in this list[1] can click the submit button.
[1] https://gerrit.ovirt.org/c/ovirt-engine/+/114130
<https://gerrit.ovirt.org/c/ovirt-engine/+/114130>
> I’d love to get to a point when we can automatically gate patches based on OST, but
it’s going slow…so for now it’s still manual.
Not sure that's enough, but it would be a step in the right direction.
Sometimes patches won't break OST but are still harmful.
sure, it’s never 100%, still better than today, especially since we started to pay more
attention to OST results it’s not only that it brings in a regression, it’s also all that
wasted time by people trying to fix OST after such patch.
Did we ever consider going fully to Test-Driven-Development? Not
certain if there are studies/methods to calculate/approximate the
expected extra work this will require. Assuming you want eventually
all developers to also know well-enough OST (in addition to their
specific expertise) and be comfortable patching it, it might make
sense.
I think anything more involved would be complicated by the slowness. It’s better than
before, but still ~35 minutes at best for basic suite. And it’s fragile so not really that
simple to add a test without either breaking it or making it too isolated - too slow for
practical use. There are always unit tests of course, that’s a separate matter...
Thanks,
michal
Best regards,
—
Didi
[1] https://gerrit.ovirt.org/#/admin/groups/63,members