Re: [openstack-dev] [TripleO] Set WIP for stale patches?
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?
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?
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?
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?
On 18/09/14 00:29, James Polley wrote: On Wed, Sep 17, 2014 at 6:26 PM, mar...@redhat.com mailto:mar...@redhat.com mandr...@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 mailto: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?
On Thu, Sep 18, 2014 at 5:21 PM, mar...@redhat.com mandr...@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?
-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 jonpaul.sulli...@hp.com 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?
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?
On 18 Sep 2014, at 6:06 pm, Sullivan, Jon Paul jonpaul.sulli...@hp.com 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 jonpaul.sulli...@hp.com 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?
Sullivan, Jon Paul jonpaul.sulli...@hp.com 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?
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:mar...@redhat.com mandr...@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 mailto: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?
- 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
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. ___ 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?
-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?
On Wed, Sep 17, 2014 at 6:26 PM, mar...@redhat.com 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 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?
On Thu, Sep 18, 2014 at 8:24 AM, James E. Blair cor...@inaugust.com wrote: Sullivan, Jon Paul jonpaul.sulli...@hp.com 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-2title=TripleO+InboxMy+Patches+Requiring+Attention=owner%3Aself+%28label%3AVerified-1%252cjenkins+OR+label%3ACode-Review-1%29TripleO+Specs=NOT+owner%3Aself+project%3Aopenstack%2Ftripleo-specsNeeds+Approval=label%3AVerified%3E%3D1%252cjenkins+NOT+owner%3Aself+label%3ACode-Review%3E%3D2+NOT+label%3ACode-Review-15+Days+Without+Feedback=label%3AVerified%3E%3D1%252cjenkins+NOT+owner%3Aself+NOT+project%3Aopenstack%2Ftripleo-specs+NOT+label%3ACode-Review%3C%3D2+age%3A5dNo+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%3A50Other=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
