Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Hayes, Graham]

On 04/04/2016 23:20, Carl Baldwin wrote:
> On Mon, Apr 4, 2016 at 3:22 PM, Doug Wiegley
>  wrote:
>> I don’t know, -1 really means, “there is something wrong, the submitter
>> should fix it and clear the slate.”  Whereas -2 has two meanings.  The first
>> is “procedural block”, and the second is “f*** you.”
>>
>> I really don’t see a reason not to use the procedural block as a procedural
>> block. Are you not trusting the other cores to remove them or something?
>> It’s literally what it’s there for.
>
> I'm not complaining.  I've had plenty of these -2s and I understanding
> the reason behind it.  But, I thought I'd chime in.
>
> I interpret a -2 on a patch as a procedural block because of something
> related to the patch.  It is awkward as a procedural block when it is
> being applied due to circumstances that have nothing to do with the
> patch itself and the only person who can remove the block is the
> person who applied it in the first place.  That person might get
> distracted, leave for the week-end, go on vacation, etc.
>
> Would it be nice if the project itself had an easy procedural block?
> A single switch that turns off entering the gate queue for the entire
> project?  Wouldn't it also be nice if the switch could be toggled by
> any one of a group responsible for it?  I think it would be nice but
> I'm not sure how it could be easily implemented.
>
> Carl

See my response earlier. if it is too "over engineered" there is
variations that could work (e.g. a script with shared creds / some sort
of side repo that triggers a jenkins job to run it) - I just suggested
a bot as it seemed easier.

On a side note, I have always thought that our overloaded use of -2 to
procedurally block things was a bit weird. A new label seems like a
much better idea.

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Carl Baldwin]

On Mon, Apr 4, 2016 at 3:22 PM, Doug Wiegley
 wrote:
> I don’t know, -1 really means, “there is something wrong, the submitter
> should fix it and clear the slate.”  Whereas -2 has two meanings.  The first
> is “procedural block”, and the second is “f*** you.”
>
> I really don’t see a reason not to use the procedural block as a procedural
> block. Are you not trusting the other cores to remove them or something?
> It’s literally what it’s there for.

I'm not complaining.  I've had plenty of these -2s and I understanding
the reason behind it.  But, I thought I'd chime in.

I interpret a -2 on a patch as a procedural block because of something
related to the patch.  It is awkward as a procedural block when it is
being applied due to circumstances that have nothing to do with the
patch itself and the only person who can remove the block is the
person who applied it in the first place.  That person might get
distracted, leave for the week-end, go on vacation, etc.

Would it be nice if the project itself had an easy procedural block?
A single switch that turns off entering the gate queue for the entire
project?  Wouldn't it also be nice if the switch could be toggled by
any one of a group responsible for it?  I think it would be nice but
I'm not sure how it could be easily implemented.

Carl

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Doug Wiegley]

> On Apr 4, 2016, at 10:37 AM, Armando M.  wrote:
> 
> 
> 
> On 4 April 2016 at 09:22, Ihar Hrachyshka  > wrote:
> Armando M. > wrote:
> 
> 
> 
> On 4 April 2016 at 09:01, Ihar Hrachyshka  > wrote:
> Hi all,
> 
> I noticed that often times we go and -2 all the patches in the review queue 
> on every neutron specific gate breakage spotted. This is allegedly done to 
> make sure that nothing known to be broken land in merge gate until we fix the 
> breakage on our side.
> 
> This is not allegedly done. When I do it, I put a comment next to my action.
> 
> 
> 
> While I share the goal of not resetting the gate if we can avoid it, I find 
> the way we do it a bit too aggressive. Especially considering that often 
> times those -2 votes sit there not cleared even days after the causing 
> breakage is fixed, needlessly blocking patches landing.
> 
> That's a blatant lie: I am aggressive at putting -2s as well as removing 
> them. Other changes for those the -2 stick is probably because they aren't 
> worth the hassle. We've been also in feature freeze so slowing things down 
> should have hurt anyway.
> 
> 
> I suggest we either make sure that we remove those -2 votes right after gate 
> fixes land, or we use other means to communicate to core reviewers that there 
> is a time window when nothing should land in the merge queue.
> 
> Initially I tried sending emails ahead of time alerting for gate breakages, 
> but that doesn't work for obvious reasons: there is a lag that can still be 
> fatal.
> 
> On the specific circumstance, gate broke on Friday late afternoon PDT. It 
> didn't seem that was anything critical worth merging at all cost that 
> couldn't wait until Monday morning and I didn't bother check that things 
> merged safely in the middle of my weekend.
> 
> Yeah, but it’s already up to two working days in some places.
> 
> I hear ya, but I only blocked 6 patches with one +2, none of which were 
> critical, so I really didn't see much of a disruption; that said, I 
> appreciate your note, and I'll be even more cautious next time.
>  
> 
> Note that I don’t mean you should check anything on your weekend. Instead, I 
> think we should avoid -2’s in this case and teach core reviewers to check 
> some source of gate state truth. An email would actually work as long as 
> everyone actively checks it [if for some reason people are not reading 
> openstack-dev@, let’s To: everyone in the group].
> 
> Perhaps we could try using -1, rather than -2, hoping it gets the same level 
> of attention. Having tried the entire past cycle with emails I don't see how 
> they could work at all.

I don’t know, -1 really means, “there is something wrong, the submitter should 
fix it and clear the slate.”  Whereas -2 has two meanings.  The first is 
“procedural block”, and the second is “f*** you.”

I really don’t see a reason not to use the procedural block as a procedural 
block. Are you not trusting the other cores to remove them or something? It’s 
literally what it’s there for.

Thanks,
doug



> 
> 
> 
> Ihar
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe 
> 
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev 
> 
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org 
> ?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev 
> 
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Darek Smigiel]

On 04/04/2016 11:41 AM, Armando M. wrote:



On 4 April 2016 at 09:36, Ihar Hrachyshka > wrote:


Graham > wrote:

On 04/04/2016 17:11, Ihar Hrachyshka wrote:

Hi all,

I noticed that often times we go and -2 all the patches in
the review queue
on every neutron specific gate breakage spotted. This is
allegedly done to
make sure that nothing known to be broken land in merge
gate until we fix
the breakage on our side.

While I share the goal of not resetting the gate if we can
avoid it, I find
the way we do it a bit too aggressive. Especially
considering that often
times those -2 votes sit there not cleared even days after
the causing
breakage is fixed, needlessly blocking patches landing.

I suggest we either make sure that we remove those -2
votes right after
gate fixes land, or we use other means to communicate to
core reviewers
that there is a time window when nothing should land in
the merge queue.

Thanks,
Ihar


I recently submitted https://review.openstack.org/295253 as an
idea for
designate to prioritize reviews.

Something similar could be a good solution, in conjunction
with a bot.

So, when a gate breakage starts, saying "!gate breakage" would
apply a
"-1 Procedural Block" that gets removed when "!gate fixed" was
said?

This removes the need for humans to do the removal (and try and
remember which reviews were really -2'd or they had had a -1 on)


Thanks for the idea, that’s indeed an interesting approach. It
also helps in that now any core member would be able to
consistently block project patches for merge gate, or cancel the
alert.

Armando, do you think we could try to adopt the approach? If yes,
I may look into a patch for that.


Um, not sure, it feels over-engineered.


My $0.02
Recently, we had several gate breakages, so it would be nice to have 
automatic tool to block/unblock patchsets without any problem.


I don't know if this is a good solution, but could give *power* to all 
core reviewers to immediately give "STOP" sign for all patchsets.


--
Darek





Ihar


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe:
openstack-dev-requ...@lists.openstack.org?subject:unsubscribe

http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Darek Smigiel]

One of these patchsets was mine, so I feel qualified to send a response :)

On 04/04/2016 12:06 PM, Armando M. wrote:



On 4 April 2016 at 09:51, Ihar Hrachyshka > wrote:


Doug Wiegley > wrote:

On Apr 4, 2016, at 10:22 AM, Ihar Hrachyshka
> wrote:

Armando M. >
wrote:

On 4 April 2016 at 09:01, Ihar Hrachyshka
> wrote:
Hi all,

I noticed that often times we go and -2 all the
patches in the review queue on every neutron specific
gate breakage spotted. This is allegedly done to make
sure that nothing known to be broken land in merge
gate until we fix the breakage on our side.

This is not allegedly done. When I do it, I put a
comment next to my action.



While I share the goal of not resetting the gate if we
can avoid it, I find the way we do it a bit too
aggressive. Especially considering that often times
those -2 votes sit there not cleared even days after
the causing breakage is fixed, needlessly blocking
patches landing.

That's a blatant lie: I am aggressive at putting -2s
as well as removing them. Other changes for those the
-2 stick is probably because they aren't worth the
hassle. We've been also in feature freeze so slowing
things down should have hurt anyway.


I suggest we either make sure that we remove those -2
votes right after gate fixes land, or we use other
means to communicate to core reviewers that there is a
time window when nothing should land in the merge queue.

Initially I tried sending emails ahead of time
alerting for gate breakages, but that doesn't work for
obvious reasons: there is a lag that can still be fatal.



Emails don't work. Or work just occasionally.
Openstack Dev mailing list is pretty crowded, so sometimes to read 
everything, takes hours. In this situation, important message can be 
easily skipped.




On the specific circumstance, gate broke on Friday
late afternoon PDT. It didn't seem that was anything
critical worth merging at all cost that couldn't wait
until Monday morning and I didn't bother check that
things merged safely in the middle of my weekend.


Yeah, but it’s already up to two working days in some places.


Not that -2’s sitting around is good, but what is so urgent
that two days affects the overall flow of things, and didn’t
get escalated?  Review chains can address collaboration
issues.  Monster syntax churns with lots of conflicts get more
annoying, but they’re annoying for everyone anyway. The worst
part of two days with a -2 is the fact that no one will look
at it and give feedback during that time period, IMO, not that
it takes longer to merge.  Velocity is about throughput, not
latency.


It is definitely not the end of the world. The process of -2
cancellation is just non-transparent, and I am not sure whether I
need to reach the vote owner to remove it, or it will just
magically vanish. I had inconsistent experiences with freezing
-2’s in OpenStack.


If the vote doesn't magically vanish when you expect to, you can 
simply reach out the person. When has that become a problem, 
especially when that person is usually available on irc and generally 
very responsive?


The -2 keeps you on your toes and aware of the state of the gate, 
which to me is a good thing :)


I'll shortly describe the situation.
My patchset got approved. It had +W and gate pre-approved it, but failed 
on final merge. So at the end landed as +2, +W and -2 from gate.


I didn't know what happened until I've seen Armando's "-2" with 
explanation. Even though I'm trying to be proactive on IRC channel about 
possible gate problems.


So it's definitely good method to "be aware". But, in the same time it 
was very strange to me. I had everything prepared to be merged, but it 
didn't got merge.




Landing a patch earlier lowers the chance of git conflict for
other patches being crafted in parallel with it; it also removes
the need for a core reviewer to get back to it and +W later, in
case enough +2 votes are there. 



I like the idea of adopting -1 instead of -2 and looking whether
it still works for the initial goal 

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Armando M.]

On 4 April 2016 at 09:51, Ihar Hrachyshka  wrote:

> Doug Wiegley  wrote:
>
> On Apr 4, 2016, at 10:22 AM, Ihar Hrachyshka  wrote:
>>>
>>> Armando M.  wrote:
>>>
>>> On 4 April 2016 at 09:01, Ihar Hrachyshka  wrote:
 Hi all,

 I noticed that often times we go and -2 all the patches in the review
 queue on every neutron specific gate breakage spotted. This is allegedly
 done to make sure that nothing known to be broken land in merge gate until
 we fix the breakage on our side.

 This is not allegedly done. When I do it, I put a comment next to my
 action.



 While I share the goal of not resetting the gate if we can avoid it, I
 find the way we do it a bit too aggressive. Especially considering that
 often times those -2 votes sit there not cleared even days after the
 causing breakage is fixed, needlessly blocking patches landing.

 That's a blatant lie: I am aggressive at putting -2s as well as
 removing them. Other changes for those the -2 stick is probably because
 they aren't worth the hassle. We've been also in feature freeze so slowing
 things down should have hurt anyway.


 I suggest we either make sure that we remove those -2 votes right after
 gate fixes land, or we use other means to communicate to core reviewers
 that there is a time window when nothing should land in the merge queue.

 Initially I tried sending emails ahead of time alerting for gate
 breakages, but that doesn't work for obvious reasons: there is a lag that
 can still be fatal.

 On the specific circumstance, gate broke on Friday late afternoon PDT.
 It didn't seem that was anything critical worth merging at all cost that
 couldn't wait until Monday morning and I didn't bother check that things
 merged safely in the middle of my weekend.

>>>
>>> Yeah, but it’s already up to two working days in some places.
>>>
>>
>> Not that -2’s sitting around is good, but what is so urgent that two days
>> affects the overall flow of things, and didn’t get escalated?  Review
>> chains can address collaboration issues.  Monster syntax churns with lots
>> of conflicts get more annoying, but they’re annoying for everyone anyway.
>> The worst part of two days with a -2 is the fact that no one will look at
>> it and give feedback during that time period, IMO, not that it takes longer
>> to merge.  Velocity is about throughput, not latency.
>>
>
> It is definitely not the end of the world. The process of -2 cancellation
> is just non-transparent, and I am not sure whether I need to reach the vote
> owner to remove it, or it will just magically vanish. I had inconsistent
> experiences with freezing -2’s in OpenStack.
>

If the vote doesn't magically vanish when you expect to, you can simply
reach out the person. When has that become a problem, especially when that
person is usually available on irc and generally very responsive?

The -2 keeps you on your toes and aware of the state of the gate, which to
me is a good thing :)


>
> Landing a patch earlier lowers the chance of git conflict for other
> patches being crafted in parallel with it; it also removes the need for a
> core reviewer to get back to it and +W later, in case enough +2 votes are
> there.




> I like the idea of adopting -1 instead of -2 and looking whether it still
> works for the initial goal of avoiding gate resets.
>
> btw does anyone know whether other projects apply a similar cautious
> approach when dealing with their gate breakages?
>
>

> Ihar
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Ihar Hrachyshka]

Doug Wiegley  wrote:


On Apr 4, 2016, at 10:22 AM, Ihar Hrachyshka  wrote:

Armando M.  wrote:


On 4 April 2016 at 09:01, Ihar Hrachyshka  wrote:
Hi all,

I noticed that often times we go and -2 all the patches in the review  
queue on every neutron specific gate breakage spotted. This is  
allegedly done to make sure that nothing known to be broken land in  
merge gate until we fix the breakage on our side.


This is not allegedly done. When I do it, I put a comment next to my  
action.




While I share the goal of not resetting the gate if we can avoid it, I  
find the way we do it a bit too aggressive. Especially considering that  
often times those -2 votes sit there not cleared even days after the  
causing breakage is fixed, needlessly blocking patches landing.


That's a blatant lie: I am aggressive at putting -2s as well as  
removing them. Other changes for those the -2 stick is probably because  
they aren't worth the hassle. We've been also in feature freeze so  
slowing things down should have hurt anyway.



I suggest we either make sure that we remove those -2 votes right after  
gate fixes land, or we use other means to communicate to core reviewers  
that there is a time window when nothing should land in the merge queue.


Initially I tried sending emails ahead of time alerting for gate  
breakages, but that doesn't work for obvious reasons: there is a lag  
that can still be fatal.


On the specific circumstance, gate broke on Friday late afternoon PDT.  
It didn't seem that was anything critical worth merging at all cost  
that couldn't wait until Monday morning and I didn't bother check that  
things merged safely in the middle of my weekend.


Yeah, but it’s already up to two working days in some places.


Not that -2’s sitting around is good, but what is so urgent that two days  
affects the overall flow of things, and didn’t get escalated?  Review  
chains can address collaboration issues.  Monster syntax churns with lots  
of conflicts get more annoying, but they’re annoying for everyone anyway.  
The worst part of two days with a -2 is the fact that no one will look at  
it and give feedback during that time period, IMO, not that it takes  
longer to merge.  Velocity is about throughput, not latency.


It is definitely not the end of the world. The process of -2 cancellation  
is just non-transparent, and I am not sure whether I need to reach the vote  
owner to remove it, or it will just magically vanish. I had inconsistent  
experiences with freezing -2’s in OpenStack.


Landing a patch earlier lowers the chance of git conflict for other patches  
being crafted in parallel with it; it also removes the need for a core  
reviewer to get back to it and +W later, in case enough +2 votes are there.


I like the idea of adopting -1 instead of -2 and looking whether it still  
works for the initial goal of avoiding gate resets.


btw does anyone know whether other projects apply a similar cautious  
approach when dealing with their gate breakages?


Ihar

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Armando M.]

On 4 April 2016 at 09:36, Ihar Hrachyshka  wrote:

> Graham  wrote:
>
> On 04/04/2016 17:11, Ihar Hrachyshka wrote:
>>
>>> Hi all,
>>>
>>> I noticed that often times we go and -2 all the patches in the review
>>> queue
>>> on every neutron specific gate breakage spotted. This is allegedly done
>>> to
>>> make sure that nothing known to be broken land in merge gate until we fix
>>> the breakage on our side.
>>>
>>> While I share the goal of not resetting the gate if we can avoid it, I
>>> find
>>> the way we do it a bit too aggressive. Especially considering that often
>>> times those -2 votes sit there not cleared even days after the causing
>>> breakage is fixed, needlessly blocking patches landing.
>>>
>>> I suggest we either make sure that we remove those -2 votes right after
>>> gate fixes land, or we use other means to communicate to core reviewers
>>> that there is a time window when nothing should land in the merge queue.
>>>
>>> Thanks,
>>> Ihar
>>>
>>
>> I recently submitted https://review.openstack.org/295253 as an idea for
>> designate to prioritize reviews.
>>
>> Something similar could be a good solution, in conjunction with a bot.
>>
>> So, when a gate breakage starts, saying "!gate breakage" would apply a
>> "-1 Procedural Block" that gets removed when "!gate fixed" was said?
>>
>> This removes the need for humans to do the removal (and try and
>> remember which reviews were really -2'd or they had had a -1 on)
>>
>
> Thanks for the idea, that’s indeed an interesting approach. It also helps
> in that now any core member would be able to consistently block project
> patches for merge gate, or cancel the alert.
>
> Armando, do you think we could try to adopt the approach? If yes, I may
> look into a patch for that.


Um, not sure, it feels over-engineered.


>
>
> Ihar
>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Armando M.]

On 4 April 2016 at 09:22, Ihar Hrachyshka  wrote:

> Armando M.  wrote:
>
>
>>
>> On 4 April 2016 at 09:01, Ihar Hrachyshka  wrote:
>> Hi all,
>>
>> I noticed that often times we go and -2 all the patches in the review
>> queue on every neutron specific gate breakage spotted. This is allegedly
>> done to make sure that nothing known to be broken land in merge gate until
>> we fix the breakage on our side.
>>
>> This is not allegedly done. When I do it, I put a comment next to my
>> action.
>>
>>
>>
>> While I share the goal of not resetting the gate if we can avoid it, I
>> find the way we do it a bit too aggressive. Especially considering that
>> often times those -2 votes sit there not cleared even days after the
>> causing breakage is fixed, needlessly blocking patches landing.
>>
>> That's a blatant lie: I am aggressive at putting -2s as well as removing
>> them. Other changes for those the -2 stick is probably because they aren't
>> worth the hassle. We've been also in feature freeze so slowing things down
>> should have hurt anyway.
>>
>>
>> I suggest we either make sure that we remove those -2 votes right after
>> gate fixes land, or we use other means to communicate to core reviewers
>> that there is a time window when nothing should land in the merge queue.
>>
>> Initially I tried sending emails ahead of time alerting for gate
>> breakages, but that doesn't work for obvious reasons: there is a lag that
>> can still be fatal.
>>
>> On the specific circumstance, gate broke on Friday late afternoon PDT. It
>> didn't seem that was anything critical worth merging at all cost that
>> couldn't wait until Monday morning and I didn't bother check that things
>> merged safely in the middle of my weekend.
>>
>
> Yeah, but it’s already up to two working days in some places.
>

I hear ya, but I only blocked 6 patches with one +2, none of which were
critical, so I really didn't see much of a disruption; that said, I
appreciate your note, and I'll be even more cautious next time.


>
> Note that I don’t mean you should check anything on your weekend. Instead,
> I think we should avoid -2’s in this case and teach core reviewers to check
> some source of gate state truth. An email would actually work as long as
> everyone actively checks it [if for some reason people are not reading
> openstack-dev@, let’s To: everyone in the group].


Perhaps we could try using -1, rather than -2, hoping it gets the same
level of attention. Having tried the entire past cycle with emails I don't
see how they could work at all.


>
> Ihar
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Doug Wiegley]

> On Apr 4, 2016, at 10:22 AM, Ihar Hrachyshka  wrote:
> 
> Armando M.  wrote:
> 
>> 
>> 
>> On 4 April 2016 at 09:01, Ihar Hrachyshka  wrote:
>> Hi all,
>> 
>> I noticed that often times we go and -2 all the patches in the review queue 
>> on every neutron specific gate breakage spotted. This is allegedly done to 
>> make sure that nothing known to be broken land in merge gate until we fix 
>> the breakage on our side.
>> 
>> This is not allegedly done. When I do it, I put a comment next to my action.
>> 
>> 
>> 
>> While I share the goal of not resetting the gate if we can avoid it, I find 
>> the way we do it a bit too aggressive. Especially considering that often 
>> times those -2 votes sit there not cleared even days after the causing 
>> breakage is fixed, needlessly blocking patches landing.
>> 
>> That's a blatant lie: I am aggressive at putting -2s as well as removing 
>> them. Other changes for those the -2 stick is probably because they aren't 
>> worth the hassle. We've been also in feature freeze so slowing things down 
>> should have hurt anyway.
>> 
>> 
>> I suggest we either make sure that we remove those -2 votes right after gate 
>> fixes land, or we use other means to communicate to core reviewers that 
>> there is a time window when nothing should land in the merge queue.
>> 
>> Initially I tried sending emails ahead of time alerting for gate breakages, 
>> but that doesn't work for obvious reasons: there is a lag that can still be 
>> fatal.
>> 
>> On the specific circumstance, gate broke on Friday late afternoon PDT. It 
>> didn't seem that was anything critical worth merging at all cost that 
>> couldn't wait until Monday morning and I didn't bother check that things 
>> merged safely in the middle of my weekend.
> 
> Yeah, but it’s already up to two working days in some places.

Not that -2’s sitting around is good, but what is so urgent that two days 
affects the overall flow of things, and didn’t get escalated?  Review chains 
can address collaboration issues.  Monster syntax churns with lots of conflicts 
get more annoying, but they’re annoying for everyone anyway. The worst part of 
two days with a -2 is the fact that no one will look at it and give feedback 
during that time period, IMO, not that it takes longer to merge.  Velocity is 
about throughput, not latency.

Thanks,
doug


> 
> Note that I don’t mean you should check anything on your weekend. Instead, I 
> think we should avoid -2’s in this case and teach core reviewers to check 
> some source of gate state truth. An email would actually work as long as 
> everyone actively checks it [if for some reason people are not reading 
> openstack-dev@, let’s To: everyone in the group].
> 
> Ihar
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Ihar Hrachyshka]

Graham  wrote:


On 04/04/2016 17:11, Ihar Hrachyshka wrote:

Hi all,

I noticed that often times we go and -2 all the patches in the review  
queue

on every neutron specific gate breakage spotted. This is allegedly done to
make sure that nothing known to be broken land in merge gate until we fix
the breakage on our side.

While I share the goal of not resetting the gate if we can avoid it, I  
find

the way we do it a bit too aggressive. Especially considering that often
times those -2 votes sit there not cleared even days after the causing
breakage is fixed, needlessly blocking patches landing.

I suggest we either make sure that we remove those -2 votes right after
gate fixes land, or we use other means to communicate to core reviewers
that there is a time window when nothing should land in the merge queue.

Thanks,
Ihar


I recently submitted https://review.openstack.org/295253 as an idea for
designate to prioritize reviews.

Something similar could be a good solution, in conjunction with a bot.

So, when a gate breakage starts, saying "!gate breakage" would apply a
"-1 Procedural Block" that gets removed when "!gate fixed" was said?

This removes the need for humans to do the removal (and try and
remember which reviews were really -2'd or they had had a -1 on)


Thanks for the idea, that’s indeed an interesting approach. It also helps  
in that now any core member would be able to consistently block project  
patches for merge gate, or cancel the alert.


Armando, do you think we could try to adopt the approach? If yes, I may  
look into a patch for that.


Ihar

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Ihar Hrachyshka]

Armando M.  wrote:




On 4 April 2016 at 09:01, Ihar Hrachyshka  wrote:
Hi all,

I noticed that often times we go and -2 all the patches in the review  
queue on every neutron specific gate breakage spotted. This is allegedly  
done to make sure that nothing known to be broken land in merge gate  
until we fix the breakage on our side.


This is not allegedly done. When I do it, I put a comment next to my  
action.




While I share the goal of not resetting the gate if we can avoid it, I  
find the way we do it a bit too aggressive. Especially considering that  
often times those -2 votes sit there not cleared even days after the  
causing breakage is fixed, needlessly blocking patches landing.


That's a blatant lie: I am aggressive at putting -2s as well as removing  
them. Other changes for those the -2 stick is probably because they  
aren't worth the hassle. We've been also in feature freeze so slowing  
things down should have hurt anyway.



I suggest we either make sure that we remove those -2 votes right after  
gate fixes land, or we use other means to communicate to core reviewers  
that there is a time window when nothing should land in the merge queue.


Initially I tried sending emails ahead of time alerting for gate  
breakages, but that doesn't work for obvious reasons: there is a lag that  
can still be fatal.


On the specific circumstance, gate broke on Friday late afternoon PDT. It  
didn't seem that was anything critical worth merging at all cost that  
couldn't wait until Monday morning and I didn't bother check that things  
merged safely in the middle of my weekend.


Yeah, but it’s already up to two working days in some places.

Note that I don’t mean you should check anything on your weekend. Instead,  
I think we should avoid -2’s in this case and teach core reviewers to check  
some source of gate state truth. An email would actually work as long as  
everyone actively checks it [if for some reason people are not reading  
openstack-dev@, let’s To: everyone in the group].


Ihar

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Hayes, Graham]

On 04/04/2016 17:11, Ihar Hrachyshka wrote:
> Hi all,
>
> I noticed that often times we go and -2 all the patches in the review queue
> on every neutron specific gate breakage spotted. This is allegedly done to
> make sure that nothing known to be broken land in merge gate until we fix
> the breakage on our side.
>
> While I share the goal of not resetting the gate if we can avoid it, I find
> the way we do it a bit too aggressive. Especially considering that often
> times those -2 votes sit there not cleared even days after the causing
> breakage is fixed, needlessly blocking patches landing.
>
> I suggest we either make sure that we remove those -2 votes right after
> gate fixes land, or we use other means to communicate to core reviewers
> that there is a time window when nothing should land in the merge queue.
>
> Thanks,
> Ihar
>

I recently submitted https://review.openstack.org/295253 as an idea for
designate to prioritize reviews.

Something similar could be a good solution, in conjunction with a bot.

So, when a gate breakage starts, saying "!gate breakage" would apply a
"-1 Procedural Block" that gets removed when "!gate fixed" was said?

This removes the need for humans to do the removal (and try and
remember which reviews were really -2'd or they had had a -1 on)

- Graham


__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Armando M.]

On 4 April 2016 at 09:01, Ihar Hrachyshka  wrote:

> Hi all,
>
> I noticed that often times we go and -2 all the patches in the review
> queue on every neutron specific gate breakage spotted. This is allegedly
> done to make sure that nothing known to be broken land in merge gate until
> we fix the breakage on our side.


This is not allegedly done. When I do it, I put a comment next to my action.


>


> While I share the goal of not resetting the gate if we can avoid it, I
> find the way we do it a bit too aggressive. Especially considering that
> often times those -2 votes sit there not cleared even days after the
> causing breakage is fixed, needlessly blocking patches landing.
>

That's a blatant lie: I am aggressive at putting -2s as well as removing
them. Other changes for those the -2 stick is probably because they aren't
worth the hassle. We've been also in feature freeze so slowing things down
should have hurt anyway.


>
> I suggest we either make sure that we remove those -2 votes right after
> gate fixes land, or we use other means to communicate to core reviewers
> that there is a time window when nothing should land in the merge queue.
>

Initially I tried sending emails ahead of time alerting for gate breakages,
but that doesn't work for obvious reasons: there is a lag that can still be
fatal.

On the specific circumstance, gate broke on Friday late afternoon PDT. It
didn't seem that was anything critical worth merging at all cost that
couldn't wait until Monday morning and I didn't bother check that things
merged safely in the middle of my weekend.


> Thanks,
> Ihar
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Anita Kuno]

On 04/04/2016 12:01 PM, Ihar Hrachyshka wrote:
> Hi all,
> 
> I noticed that often times we go and -2 all the patches in the review
> queue on every neutron specific gate breakage spotted. This is allegedly
> done to make sure that nothing known to be broken land in merge gate
> until we fix the breakage on our side.
> 
> While I share the goal of not resetting the gate if we can avoid it, I
> find the way we do it a bit too aggressive. Especially considering that
> often times those -2 votes sit there not cleared even days after the
> causing breakage is fixed, needlessly blocking patches landing.
> 
> I suggest we either make sure that we remove those -2 votes right after
> gate fixes land, or we use other means to communicate to core reviewers
> that there is a time window when nothing should land in the merge queue.
> 
> Thanks,
> Ihar
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Well one thing that could be done is that one of the core reviewers go
into gerrit and remove everyone else from the core review group until
merging is permitted again.

It avoids the -2'ing everything and then having to clean up, but would
really have to be socialized and accepted by everyone affected prior to
being implemented, lest it create issues of a different kind.

Thanks,
Anita.

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

[openstack-dev] [neutron] -2'ing all patches on every gate breakage

[Ihar Hrachyshka]

Hi all,

I noticed that often times we go and -2 all the patches in the review queue  
on every neutron specific gate breakage spotted. This is allegedly done to  
make sure that nothing known to be broken land in merge gate until we fix  
the breakage on our side.


While I share the goal of not resetting the gate if we can avoid it, I find  
the way we do it a bit too aggressive. Especially considering that often  
times those -2 votes sit there not cleared even days after the causing  
breakage is fixed, needlessly blocking patches landing.


I suggest we either make sure that we remove those -2 votes right after  
gate fixes land, or we use other means to communicate to core reviewers  
that there is a time window when nothing should land in the merge queue.


Thanks,
Ihar

__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev