Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-22 Thread Stefano Maffulli 
On Fri 19 Sep 2014 09:25:10 AM PDT, Jeremy Stanley wrote:
> Here we'll just have to agree to disagree. I think core reviewers
> hiding behind an automated process so that they don't have to
> confront contributors about stalled/inadequate changes is inherently
> less friendly. Clearly you feel that contributors are less likely to
> be offended if a machine tells them they need to revisit a change
> because it's impersonal and therefore without perceived human
> judgement.

I think that, if done well, having an automated process that nudges 
owners of a patch when needed could be valuable. Having to rely on 
core-reviewers to start a confrontation may also work, but they need to 
be given tools to engage gently and not piss people off. Sometimes 
leaving the hard job to machines is the right thing to do: don't we 
automatically welcome new committers? Why not nudge them if they 
disappear?

What I don't understand is how hard is to implement the automated 
workflow in newer gerrit and if it's worth pursuing it.

/stef

--
Ask and answer questions on https://ask.openstack.org

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-19 Thread Jay Dobies 

On 2014-09-18 15:21:20 -0400 (-0400), Jay Dobies wrote:

How many of the reviews that we WIP-1 will actually be revisited?

I'm sure there will be cases where a current developer forgetting
they had started on something, seeing the e-mail about the WIP-1,
and then abandoning the change.

But what about developers who have moved off the project entirely?
Is this only masking the problem of stale reviews from our review
stats and leaving the review queue to bloat?

[...]

What is "review queue bloat" in this scenario? How is a change
indefinitely left in Gerrit with "workflow -1" set any different
from a change indefinitely left in Gerrit with "abandoned" set? It's
not like we go through and purge changes from Gerrit based on these,
and they take up just as much space and other resources in either
state.


Ah, ok. I assumed the abandoned ones were reaped over time. Perhaps it's 
just a matter of me writing different searches when I want to ignore the 
workflow -1s.


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-19 Thread Jeremy Stanley 
On 2014-09-19 15:27:36 + (+), Sullivan, Jon Paul wrote:
[...]
> I think that the abandoning happening from an automated process is
> easier to accept than if it came from a person, and so less likely
> to create a poor and emotional response.

Here we'll just have to agree to disagree. I think core reviewers
hiding behind an automated process so that they don't have to
confront contributors about stalled/inadequate changes is inherently
less friendly. Clearly you feel that contributors are less likely to
be offended if a machine tells them they need to revisit a change
because it's impersonal and therefore without perceived human
judgement.

> If your personal opinion was that it wasn’t useful to your
> project, then perhaps what you are really saying is that the
> implementation of it was not configurable enough to allow
> individual projects to tailor it to their needs.
[...]

Sure. For what it's worth, I haven't said I would push back on
someone writing a reasonable implementation of this feature, but it
definitely is something I wouldn't want imposed on everyone's
workflow just because the majority of core reviewers on some subset
of projects found it easier to have changes abandoned for them.

> So the removal of the auto-abandon, imho, has increased core
> reviewer workload, increased the chance that a good change may get
> ignored for extended periods of time, and has increased the
> possibility of code committers becoming frustrated with core
> reviewers adding a wip or abandon to their patches, so a decrease
> in productivity all around. :(

I think this is an inaccurate representation of the situation. It
wasn't explicitly removed. It was a buggy hack which was effectively
unmaintainable, didn't work with modern versions of Gerrit, and
nobody felt like investing time in a new implementation of it nor
was it deemed a critical feature for which we should hold back
progress and continue to fester on a years-old Gerrit installation.
It broke and has never been fixed.

This is not the same thing as being removed, but since it's been
gone I've come to wish we had removed it rather than just living
with it until it ceased working due to bitrot. In this case, an
automated process determined that feature was no longer suitable and
abandoned functional use of it. I think it would have been more
friendly to our community if the people who were no longer
interested in maintaining that feature had explicitly removed it
instead... but then it sounds like you would assert that having the
machine abandon this feature for us was less likely to offend
anyone. ;)
-- 
Jeremy Stanley

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-19 Thread Jeremy Stanley 
On 2014-09-18 15:21:20 -0400 (-0400), Jay Dobies wrote:
> How many of the reviews that we WIP-1 will actually be revisited?
> 
> I'm sure there will be cases where a current developer forgetting
> they had started on something, seeing the e-mail about the WIP-1,
> and then abandoning the change.
> 
> But what about developers who have moved off the project entirely?
> Is this only masking the problem of stale reviews from our review
> stats and leaving the review queue to bloat?
[...]

What is "review queue bloat" in this scenario? How is a change
indefinitely left in Gerrit with "workflow -1" set any different
from a change indefinitely left in Gerrit with "abandoned" set? It's
not like we go through and purge changes from Gerrit based on these,
and they take up just as much space and other resources in either
state.
-- 
Jeremy Stanley

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread Jay Dobies 

How many of the reviews that we WIP-1 will actually be revisited?

I'm sure there will be cases where a current developer forgetting they 
had started on something, seeing the e-mail about the WIP-1, and then 
abandoning the change.


But what about developers who have moved off the project entirely? Is 
this only masking the problem of stale reviews from our review stats and 
leaving the review queue to bloat?


I honestly don't know; those are real questions, not rhetorical ones 
trying to prove a point. I'd guess the longer-running OpenStack projects 
have had to deal with this as well, and perhaps I'm overestimating just 
how many of these perpetually in limbo reviews there are.



On 09/18/2014 03:26 AM, mar...@redhat.com wrote:

On 18/09/14 00:29, James Polley wrote:



On Wed, Sep 17, 2014 at 6:26 PM, mar...@redhat.com
 mailto:mandr...@redhat.com>> wrote:

 Hi,

 as part of general housekeeping on our reviews, it was discussed at last
 week's meeting [1] that we should set workflow -1 for stale reviews
 (like gerrit used to do when I were a lad).

 The specific criteria discussed was 'items that have a -1 from a core
 but no response from author for 14 days'. This topic came up again
 during today's meeting and it wasn't clear if the intention was for
 cores to start enforcing this? So:

 Do we start setting WIP/workflow -1 for those reviews that have a -1
 from a core but no response from author for 14 days


I'm in favour of doing this; as long as we make it clear that we're
doing it to help us focus review effort on things that are under active
development - it doesn't mean we think the patch shouldn't land, it just
means we know it's not ready yet so we don't want reviewers to be
looking at it until it moves forward.

For the sake of making sure new developers don't get put off, I'd like
to see us leaving a comment explaining why we're WIPing the change and
noting that uploading a new revision will remove the WIP automatically



+1 - indeed, I'd say as part of this discussion, or if/when it comes up
as a motion for a vote in the weekly meeting, we should also put out and
agree on the 'standard' text to be used for this and stick it on the
wiki (regardless of whether this is to be implemented manually at first
and perhaps automated later),

thanks, marios

"setting workflow -1 as this review has been inactive for two weeks
following a negative review. Please see the wiki @ foo for more
information. Note that once you upload a new revision the workflow is
expected to be reset (feel free to shout on freenode/#tripleo if it isn't)."





 thanks, marios

 [1]
 
http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html

 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread James E. Blair 
"Sullivan, Jon Paul"  writes:

> This is not solely about finding reviews.  It is about pruning stale
> reviews.  I think the auto-abandon code was excellent at doing this,
> but alas, it is no more.

What's the purpose of pruning stale reviews?  I've read the IRC log of
the meeting you mentioned.  It's becoming apparent to me that this is
about making some numbers that reviewstats produces look good.

I am rather disappointed in that.

If reviewstats is not measuring what you want it to measure (eg, how
well you keep up with incoming reviews) then you should change how it
measures it.  If you want the number of open reviews to be low, then
change the definition of open reviews to what you think it should be --
don't create automated processes to WIP changes just to get your numbers
down.

The reason that we made it so that any core team member could WIP or
abandon a change was so that you could make a judgement call and say
"this change needs more work" or "this change is defunct".  You might
even use a tool to help you find those reviews and make those judgement
calls.  But no automated tool can make those decisions.  Luckily, it
does not need to.

If you want to organize your review queue, use a tool like
gerrit-dash-creator:

  https://github.com/stackforge/gerrit-dash-creator

If you want to change how stats are generated, patch reviewstats.

-Jim

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread James Polley 




On 18 Sep 2014, at 6:06 pm, Sullivan, Jon Paul  wrote:

>> -Original Message-
>> From: James E. Blair [mailto:cor...@inaugust.com]
>> Sent: 17 September 2014 23:24
>> To: OpenStack Development Mailing List (not for usage questions)
>> Subject: Re: [openstack-dev] [TripleO] Set WIP for stale patches?
>> 
>> "Sullivan, Jon Paul"  writes:
>> 
>>> I think this highlights exactly why this should be an automated
>>> process.  No errors in application, and no errors in interpretation of
>>> what has happened.
>>> 
>>> So the -1 from Jenkins was a reaction to the comment created by adding
>>> the workflow -1.  This is going to happen on all of the patches that
>>> have their workflow value altered (tests will run, result would be
>>> whatever the result of the test was, of course).
>> 
>> Jenkins only runs tests in reaction to comments if they say "recheck".
> 
> This is not my experience.
> 
> In my experience if the check results are not fresh enough the recheck is 
> automatically run.  I am not on the infra team, so without looking up code I 
> am just guessing, but my guess is that the workflow score change triggers the 
> check on the presumption that it has been approved, not accounting for the 
> recent(ish) update that move wip to the workflow score.
>
>> 
>>> But I also agree that the Jenkins vote should not be included in the
>>> determination of marking a patch WIP, but a human review should (So
>>> Code-Review and not Verified column).
>>> 
>>> And in fact, for the specific example to hand, the last Jenkins vote
>>> was actually a +1, so as I understand it should not have been marked
>>> WIP.
>> 
>> I'd like to help you see the reviews you want to see without
>> externalizing your individual workflow onto contributors.  What tool do
>> you use to find reviews?
>> 
>> If it's gerrit's webui, have you tried using the Review Inbox dashboard?
>> Here it is for the tripleo-image-elements project:
>> 
>>  https://review.openstack.org/#/projects/openstack/tripleo-image-
>> elements,dashboards/important-changes:review-inbox-dashboard
>> 
>> If you would prefer something else, we can customize those dashboards to
>> do whatever you want, including ignoring changes that have not been
>> updated in 2 weeks.
> 
> This is not solely about finding reviews.  It is about pruning stale reviews. 
>  I think the auto-abandon code was excellent at doing this, but alas, it is 
> no more. 

The only reason i'm aware of for pruning apparently stale reviews is to make it 
easier to find and review things that are being actively worked on.

Do you have other reasons for wanting stale reviews pruned?
> 
>> 
>> -Jim
>> 
>> ___
>> OpenStack-dev mailing list
>> OpenStack-dev@lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> Thanks, 
> Jon-Paul Sullivan ☺ Cloud Services - @hpcloud
> 
> Postal Address: Hewlett-Packard Galway Limited, Ballybrit Business Park, 
> Galway.
> Registered Office: Hewlett-Packard Galway Limited, 63-74 Sir John Rogerson's 
> Quay, Dublin 2. 
> Registered Number: 361933
> 
> The contents of this message and any attachments to it are confidential and 
> may be legally privileged. If you have received this message in error you 
> should delete it from your system immediately and advise the sender.
> 
> To any recipient of this message within HP, unless otherwise stated, you 
> should consider this message and attachments as "HP CONFIDENTIAL".
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread Jeremy Stanley 
On 2014-09-18 08:06:11 + (+), Sullivan, Jon Paul wrote:
> In my experience if the check results are not fresh enough the
> recheck is automatically run.  I am not on the infra team, so
> without looking up code I am just guessing, but my guess is that
> the workflow score change triggers the check on the presumption
> that it has been approved, not accounting for the recent(ish)
> update that move wip to the workflow score.

We turned off that behavior a couple months ago when the merge-check
pseudo-job was implemented to automatically -1 any open changes with
merge conflicts each time a new change merges to their target
branch. This covered the majority of the problems identified by the
freshness check, but without using any of our worker pool.

> This is not solely about finding reviews.  It is about pruning
> stale reviews.  I think the auto-abandon code was excellent at
> doing this, but alas, it is no more. 

I think it was excellent at arbitrarily abandoning open changes
which happened to meet a poorly-thought-out set of criteria. I'm
personally quite glad it broke and we didn't waste time
reimplementing something similar for new Gerrit versions.
-- 
Jeremy Stanley

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread Sullivan, Jon Paul 
> -Original Message-
> From: James E. Blair [mailto:cor...@inaugust.com]
> Sent: 17 September 2014 23:24
> To: OpenStack Development Mailing List (not for usage questions)
> Subject: Re: [openstack-dev] [TripleO] Set WIP for stale patches?
> 
> "Sullivan, Jon Paul"  writes:
> 
> > I think this highlights exactly why this should be an automated
> > process.  No errors in application, and no errors in interpretation of
> > what has happened.
> >
> > So the -1 from Jenkins was a reaction to the comment created by adding
> > the workflow -1.  This is going to happen on all of the patches that
> > have their workflow value altered (tests will run, result would be
> > whatever the result of the test was, of course).
> 
> Jenkins only runs tests in reaction to comments if they say "recheck".

This is not my experience.

In my experience if the check results are not fresh enough the recheck is 
automatically run.  I am not on the infra team, so without looking up code I am 
just guessing, but my guess is that the workflow score change triggers the 
check on the presumption that it has been approved, not accounting for the 
recent(ish) update that move wip to the workflow score.

> 
> > But I also agree that the Jenkins vote should not be included in the
> > determination of marking a patch WIP, but a human review should (So
> > Code-Review and not Verified column).
> >
> > And in fact, for the specific example to hand, the last Jenkins vote
> > was actually a +1, so as I understand it should not have been marked
> > WIP.
> 
> I'd like to help you see the reviews you want to see without
> externalizing your individual workflow onto contributors.  What tool do
> you use to find reviews?
> 
> If it's gerrit's webui, have you tried using the Review Inbox dashboard?
> Here it is for the tripleo-image-elements project:
> 
>   https://review.openstack.org/#/projects/openstack/tripleo-image-
> elements,dashboards/important-changes:review-inbox-dashboard
> 
> If you would prefer something else, we can customize those dashboards to
> do whatever you want, including ignoring changes that have not been
> updated in 2 weeks.

This is not solely about finding reviews.  It is about pruning stale reviews.  
I think the auto-abandon code was excellent at doing this, but alas, it is no 
more. 

> 
> -Jim
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Thanks, 
Jon-Paul Sullivan ☺ Cloud Services - @hpcloud

Postal Address: Hewlett-Packard Galway Limited, Ballybrit Business Park, Galway.
Registered Office: Hewlett-Packard Galway Limited, 63-74 Sir John Rogerson's 
Quay, Dublin 2. 
Registered Number: 361933
 
The contents of this message and any attachments to it are confidential and may 
be legally privileged. If you have received this message in error you should 
delete it from your system immediately and advise the sender.

To any recipient of this message within HP, unless otherwise stated, you should 
consider this message and attachments as "HP CONFIDENTIAL".
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread James Polley 
On Thu, Sep 18, 2014 at 5:21 PM, mar...@redhat.com 
wrote:

> On 17/09/14 16:40, Charles Crouch wrote:
> >
> >
> > - Original Message -
> >> Hi,
> >>
> >> as part of general housekeeping on our reviews, it was discussed at last
> >> week's meeting [1] that we should set workflow -1 for stale reviews
> >> (like gerrit used to do when I were a lad).
> >>
> >> The specific criteria discussed was 'items that have a -1 from a core
> >> but no response from author for 14 days'. This topic came up again
> >> during today's meeting and it wasn't clear if the intention was for
> >> cores to start enforcing this? So:
> >>
> >> Do we start setting WIP/workflow -1 for those reviews that have a -1
> >> from a core but no response from author for 14 days
> >>
> >> thanks, marios
> >>
> >> [1]
> >>
> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html
> >
> > So it looks like this has already started..
> >
> > https://review.openstack.org/#/c/105275/
> >
> > I think we need to document on the wiki *precisely* the criteria for
> setting
> > WIP/workflow -1. For example that review above has a Jenkins failure but
> no
> > core reviews at all.
>
> +1 on being precise - another case in point:
>
> https://review.openstack.org/#/c/102304/
>
> this review has a *-2* from core, which seems to 'stick' for future
> revisions; the last revision is from > 14 days ago, so does this fulfill
> the criteria (I'd say it doesn't)?
>
> Not sure if you were also suggesting that -1 from Jenkins also counts,
> which imo makes sense...
>
> 'items that have a -1 from core or jenkins, or a -2 from core on the
> latest revision, and no response from author for 14 days'
>
> Really this needs to be put as a motion and voted on in the next meeting,
>

My understanding has always been that we don't make decisions based on
votes on IRC meetings, because it's hard to get a time for the meeting that
allows the whole team to be easily present. I wouldn't feel comfortable
doing this at the US-timezone meeting as it excludes most of APAC; nor at
the EU-timezone meeting as it excludes most of the US.

If we're looking for consensus, I'd suggest we use Gerrit to collect votes.
We could model this is a change to the CONTRIBUTING.rst[1], or perhaps we
could draft a spec around the expected workflow (perhaps formalising some
of our expectations around cores averaging 3 reviews/workday + 1 spec
review/workday at the same time?

But perhaps I'm overthinking and making this far more formal than it has to
be. We've had a fair bit of discussion on the list and we seem to be
broadly in agreement; perhaps we just need someone to propose some details
and see if we can get consensus here.

[1] What's that you say? We don't have a CONTRIBUTING.rst? One second, let
me fix that *tap tap tap* We will as soon as
https://review.openstack.org/#/c/122350/ lands!


> thanks, marios
>
>
> >
> >>
> >> ___
> >> OpenStack-dev mailing list
> >> OpenStack-dev@lists.openstack.org
> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >>
> >
> > ___
> > OpenStack-dev mailing list
> > OpenStack-dev@lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
>
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread mar...@redhat.com 
On 18/09/14 00:29, James Polley wrote:
> 
> 
> On Wed, Sep 17, 2014 at 6:26 PM, mar...@redhat.com
>   > wrote:
> 
> Hi,
> 
> as part of general housekeeping on our reviews, it was discussed at last
> week's meeting [1] that we should set workflow -1 for stale reviews
> (like gerrit used to do when I were a lad).
> 
> The specific criteria discussed was 'items that have a -1 from a core
> but no response from author for 14 days'. This topic came up again
> during today's meeting and it wasn't clear if the intention was for
> cores to start enforcing this? So:
> 
> Do we start setting WIP/workflow -1 for those reviews that have a -1
> from a core but no response from author for 14 days
> 
> 
> I'm in favour of doing this; as long as we make it clear that we're
> doing it to help us focus review effort on things that are under active
> development - it doesn't mean we think the patch shouldn't land, it just
> means we know it's not ready yet so we don't want reviewers to be
> looking at it until it moves forward.
> 
> For the sake of making sure new developers don't get put off, I'd like
> to see us leaving a comment explaining why we're WIPing the change and
> noting that uploading a new revision will remove the WIP automatically
> 

+1 - indeed, I'd say as part of this discussion, or if/when it comes up
as a motion for a vote in the weekly meeting, we should also put out and
agree on the 'standard' text to be used for this and stick it on the
wiki (regardless of whether this is to be implemented manually at first
and perhaps automated later),

thanks, marios

"setting workflow -1 as this review has been inactive for two weeks
following a negative review. Please see the wiki @ foo for more
information. Note that once you upload a new revision the workflow is
expected to be reset (feel free to shout on freenode/#tripleo if it isn't)."



> 
> thanks, marios
> 
> [1]
> 
> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> 
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 
> 
> 
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-18 Thread mar...@redhat.com 
On 17/09/14 16:40, Charles Crouch wrote:
> 
> 
> - Original Message -
>> Hi,
>>
>> as part of general housekeeping on our reviews, it was discussed at last
>> week's meeting [1] that we should set workflow -1 for stale reviews
>> (like gerrit used to do when I were a lad).
>>
>> The specific criteria discussed was 'items that have a -1 from a core
>> but no response from author for 14 days'. This topic came up again
>> during today's meeting and it wasn't clear if the intention was for
>> cores to start enforcing this? So:
>>
>> Do we start setting WIP/workflow -1 for those reviews that have a -1
>> from a core but no response from author for 14 days
>>
>> thanks, marios
>>
>> [1]
>> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html
> 
> So it looks like this has already started..
> 
> https://review.openstack.org/#/c/105275/
> 
> I think we need to document on the wiki *precisely* the criteria for setting 
> WIP/workflow -1. For example that review above has a Jenkins failure but no
> core reviews at all.

+1 on being precise - another case in point:

https://review.openstack.org/#/c/102304/

this review has a *-2* from core, which seems to 'stick' for future
revisions; the last revision is from > 14 days ago, so does this fulfill
the criteria (I'd say it doesn't)?

Not sure if you were also suggesting that -1 from Jenkins also counts,
which imo makes sense...

'items that have a -1 from core or jenkins, or a -2 from core on the
latest revision, and no response from author for 14 days'

Really this needs to be put as a motion and voted on in the next meeting,

thanks, marios


> 
>>
>> ___
>> OpenStack-dev mailing list
>> OpenStack-dev@lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-17 Thread James Polley 
On Thu, Sep 18, 2014 at 8:24 AM, James E. Blair  wrote:

> "Sullivan, Jon Paul"  writes:
>
> > I think this highlights exactly why this should be an automated
> > process.  No errors in application, and no errors in interpretation of
> > what has happened.
> >
> > So the -1 from Jenkins was a reaction to the comment created by adding
> > the workflow -1.  This is going to happen on all of the patches that
> > have their workflow value altered (tests will run, result would be
> > whatever the result of the test was, of course).
>
> Jenkins only runs tests in reaction to comments if they say "recheck".
>
> > But I also agree that the Jenkins vote should not be included in the
> > determination of marking a patch WIP, but a human review should (So
> > Code-Review and not Verified column).
> >
> > And in fact, for the specific example to hand, the last Jenkins vote
> > was actually a +1, so as I understand it should not have been marked
> > WIP.
>
> I'd like to help you see the reviews you want to see without
> externalizing your individual workflow onto contributors.  What tool do
> you use to find reviews?
>

We updated our wiki back in June to point people at:

https://review.openstack.org/#/dashboard/?foreach=%28project%3Aopenstack%2Ftripleo-incubator+OR+project%3Aopenstack%2Ftripleo-image-elements+OR+project%3Aopenstack%2Ftripleo-heat-templates+OR+project%3Aopenstack%2Ftripleo-specs+OR+project%3Aopenstack%2Fos-apply-config+OR+project%3Aopenstack%2Fos-collect-config+OR+project%3Aopenstack%2Fos-refresh-config+OR+project%3Aopenstack%2Fos-cloud-config+OR+project%3Aopenstack%2Fdiskimage-builder+OR+project%3Aopenstack%2Fdib-utils+OR+project%3Aopenstack-infra%2Ftripleo-ci+OR+project%3Aopenstack%2Ftuskar+OR+project%3Aopenstack%2Fpython-tuskarclient%29+status%3Aopen+NOT+label%3AWorkflow%3C%3D-1+NOT+label%3ACode-Review%3C%3D-2&title=TripleO+Inbox&My+Patches+Requiring+Attention=owner%3Aself+%28label%3AVerified-1%252cjenkins+OR+label%3ACode-Review-1%29&TripleO+Specs=NOT+owner%3Aself+project%3Aopenstack%2Ftripleo-specs&Needs+Approval=label%3AVerified%3E%3D1%252cjenkins+NOT+owner%3Aself+label%3ACode-Review%3E%3D2+NOT+label%3ACode-Review-1&5+Days+Without+Feedback=label%3AVerified%3E%3D1%252cjenkins+NOT+owner%3Aself+NOT+project%3Aopenstack%2Ftripleo-specs+NOT+label%3ACode-Review%3C%3D2+age%3A5d&No+Negative+Feedback=label%3AVerified%3E%3D1%252cjenkins+NOT+owner%3Aself+NOT+project%3Aopenstack%2Ftripleo-specs+NOT+label%3ACode-Review%3C%3D-1+NOT+label%3ACode-Review%3E%3D2+limit%3A50&Other=label%3AVerified%3E%3D1%252cjenkins+NOT+owner%3Aself+NOT+project%3Aopenstack%2Ftripleo-specs+label%3ACode-Review-1+limit%3A20

It's probably not the only thing people use, but it should give you an idea
of what we'd find useful.

>
> If it's gerrit's webui, have you tried using the Review Inbox dashboard?
> Here it is for the tripleo-image-elements project:
>
>
> https://review.openstack.org/#/projects/openstack/tripleo-image-elements,dashboards/important-changes:review-inbox-dashboard


That gave me an error: Error in operator label:Code-Review>=0,self

After trying a few times I realised I hadn't logged in yet. After logging
in, it works. It'd be nice if it triggered the login sequence instead of
just giving me an error.


>
> If you would prefer something else, we can customize those dashboards to
> do whatever you want, including ignoring changes that have not been
> updated in 2 weeks.
>

I'd love to be able to sort by least-recently-updated, to encourage a FIFO
pattern. At present, even on our dashboard above, the best we've been able
to do is highlight things that are (for instance) >2 weeks without review -
but even then, the things that are just barely two weeks without review sit
at the top, while things that have been 60 days or more without review are
hidden further down.


>
> -Jim
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-17 Thread James E. Blair 
"Sullivan, Jon Paul"  writes:

> I think this highlights exactly why this should be an automated
> process.  No errors in application, and no errors in interpretation of
> what has happened.
>
> So the -1 from Jenkins was a reaction to the comment created by adding
> the workflow -1.  This is going to happen on all of the patches that
> have their workflow value altered (tests will run, result would be
> whatever the result of the test was, of course).

Jenkins only runs tests in reaction to comments if they say "recheck".

> But I also agree that the Jenkins vote should not be included in the
> determination of marking a patch WIP, but a human review should (So
> Code-Review and not Verified column).
>
> And in fact, for the specific example to hand, the last Jenkins vote
> was actually a +1, so as I understand it should not have been marked
> WIP.

I'd like to help you see the reviews you want to see without
externalizing your individual workflow onto contributors.  What tool do
you use to find reviews?

If it's gerrit's webui, have you tried using the Review Inbox dashboard?
Here it is for the tripleo-image-elements project:

  
https://review.openstack.org/#/projects/openstack/tripleo-image-elements,dashboards/important-changes:review-inbox-dashboard

If you would prefer something else, we can customize those dashboards to
do whatever you want, including ignoring changes that have not been
updated in 2 weeks.

-Jim

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-17 Thread James Polley 
On Wed, Sep 17, 2014 at 6:26 PM, mar...@redhat.com 
wrote:

> Hi,
>
> as part of general housekeeping on our reviews, it was discussed at last
> week's meeting [1] that we should set workflow -1 for stale reviews
> (like gerrit used to do when I were a lad).
>
> The specific criteria discussed was 'items that have a -1 from a core
> but no response from author for 14 days'. This topic came up again
> during today's meeting and it wasn't clear if the intention was for
> cores to start enforcing this? So:
>
> Do we start setting WIP/workflow -1 for those reviews that have a -1
> from a core but no response from author for 14 days
>

I'm in favour of doing this; as long as we make it clear that we're doing
it to help us focus review effort on things that are under active
development - it doesn't mean we think the patch shouldn't land, it just
means we know it's not ready yet so we don't want reviewers to be looking
at it until it moves forward.

For the sake of making sure new developers don't get put off, I'd like to
see us leaving a comment explaining why we're WIPing the change and noting
that uploading a new revision will remove the WIP automatically

>
> thanks, marios
>
> [1]
>
> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html
>
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-17 Thread Sullivan, Jon Paul 
> -Original Message-
> From: Derek Higgins [mailto:der...@redhat.com]
> Sent: 17 September 2014 14:49
> To: OpenStack Development Mailing List (not for usage questions)
> Subject: Re: [openstack-dev] [TripleO] Set WIP for stale patches?
> 
> On 17/09/14 14:40, Charles Crouch wrote:
> >
> >
> > - Original Message -
> >> Hi,
> >>
> >> as part of general housekeeping on our reviews, it was discussed at
> >> last week's meeting [1] that we should set workflow -1 for stale
> >> reviews (like gerrit used to do when I were a lad).
> >>
> >> The specific criteria discussed was 'items that have a -1 from a core
> >> but no response from author for 14 days'. This topic came up again
> >> during today's meeting and it wasn't clear if the intention was for
> >> cores to start enforcing this? So:
> >>
> >> Do we start setting WIP/workflow -1 for those reviews that have a -1
> >> from a core but no response from author for 14 days
> >>
> >> thanks, marios
> >>
> >> [1]
> >> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-
> >> 09-19.04.log.html
> >
> > So it looks like this has already started..
> >
> > https://review.openstack.org/#/c/105275/
> >
> > I think we need to document on the wiki *precisely* the criteria for
> > setting WIP/workflow -1.
> Yup, we definitely should
> 
> > For example that review above has a Jenkins failure but no core
> > reviews at all.
> FWIW I reckon a jenkins -1 should also start the 2 week clock but in the
> case you've linked the -1 was only 2 days ago so should have remained
> untouched.

I think this highlights exactly why this should be an automated process.  No 
errors in application, and no errors in interpretation of what has happened.

So the -1 from Jenkins was a reaction to the comment created by adding the 
workflow -1.  This is going to happen on all of the patches that have their 
workflow value altered (tests will run, result would be whatever the result of 
the test was, of course).

But I also agree that the Jenkins vote should not be included in the 
determination of marking a patch WIP, but a human review should (So Code-Review 
and not Verified column).

And in fact, for the specific example to hand, the last Jenkins vote was 
actually a +1, so as I understand it should not have been marked WIP.

> 
> >
> >>
> >> ___
> >> OpenStack-dev mailing list
> >> OpenStack-dev@lists.openstack.org
> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >>
> >
> > ___
> > OpenStack-dev mailing list
> > OpenStack-dev@lists.openstack.org
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
> 
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Thanks, 
Jon-Paul Sullivan ☺ Cloud Services - @hpcloud

Postal Address: Hewlett-Packard Galway Limited, Ballybrit Business Park, Galway.
Registered Office: Hewlett-Packard Galway Limited, 63-74 Sir John Rogerson's 
Quay, Dublin 2. 
Registered Number: 361933
 
The contents of this message and any attachments to it are confidential and may 
be legally privileged. If you have received this message in error you should 
delete it from your system immediately and advise the sender.

To any recipient of this message within HP, unless otherwise stated, you should 
consider this message and attachments as "HP CONFIDENTIAL".
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-17 Thread Derek Higgins 
On 17/09/14 14:40, Charles Crouch wrote:
> 
> 
> - Original Message -
>> Hi,
>>
>> as part of general housekeeping on our reviews, it was discussed at last
>> week's meeting [1] that we should set workflow -1 for stale reviews
>> (like gerrit used to do when I were a lad).
>>
>> The specific criteria discussed was 'items that have a -1 from a core
>> but no response from author for 14 days'. This topic came up again
>> during today's meeting and it wasn't clear if the intention was for
>> cores to start enforcing this? So:
>>
>> Do we start setting WIP/workflow -1 for those reviews that have a -1
>> from a core but no response from author for 14 days
>>
>> thanks, marios
>>
>> [1]
>> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html
> 
> So it looks like this has already started..
> 
> https://review.openstack.org/#/c/105275/
> 
> I think we need to document on the wiki *precisely* the criteria for setting 
> WIP/workflow -1.
Yup, we definitely should

> For example that review above has a Jenkins failure but no
> core reviews at all.
FWIW I reckon a jenkins -1 should also start the 2 week clock but in the
case you've linked the -1 was only 2 days ago so should have remained
untouched.

> 
>>
>> ___
>> OpenStack-dev mailing list
>> OpenStack-dev@lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [TripleO] Set WIP for stale patches?

2014-09-17 Thread Charles Crouch 


- Original Message -
> Hi,
> 
> as part of general housekeeping on our reviews, it was discussed at last
> week's meeting [1] that we should set workflow -1 for stale reviews
> (like gerrit used to do when I were a lad).
> 
> The specific criteria discussed was 'items that have a -1 from a core
> but no response from author for 14 days'. This topic came up again
> during today's meeting and it wasn't clear if the intention was for
> cores to start enforcing this? So:
> 
> Do we start setting WIP/workflow -1 for those reviews that have a -1
> from a core but no response from author for 14 days
> 
> thanks, marios
> 
> [1]
> http://eavesdrop.openstack.org/meetings/tripleo/2014/tripleo.2014-09-09-19.04.log.html

So it looks like this has already started..

https://review.openstack.org/#/c/105275/

I think we need to document on the wiki *precisely* the criteria for setting 
WIP/workflow -1. For example that review above has a Jenkins failure but no
core reviews at all.

> 
> ___
> OpenStack-dev mailing list
> OpenStack-dev@lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev