Hi Adrien, Thanks for the review!
In the literal sense, “cancelling patch review” will transition an issue's status from “Patch Available” to “Open”. Since the Yetus PreCommit-Admin Jenkins job only queues an issue’s patches to be reviewed when the issue is in “Patch Available” status, this will result in further attached patches not being reviewed. When you asked about the implications of cancelling patch review, maybe you meant: "why would somebody want to cancel patch review?" One case I can think of: patch review finds problems that need fixing, but the contributor wants to post WIP patches with only partial fixes that aren’t ready yet for review. Another case: the contributor is tired of seeing a bunch of Solr failures that have nothing to do with their patch, and they opt to run “ant precommit” instead. -- Steve www.lucidworks.com > On Jun 21, 2018, at 3:35 AM, Adrien Grand <[email protected]> wrote: > > Thanks Steve for taking care of it, the proposal makes sense to me. I'm not > 100% sure what cancelling patch reviews implies, could you give examples of > when we should do it? > > Le mer. 20 juin 2018 à 23:52, Steve Rowe <[email protected]> a écrit : > Hi David, > > Thanks for the review! > > I agree that it would be nice to be able to (re-)enable patch review > independently from uploading a (new) patch. I’ll go mention your idea on > INFRA-16094. > > -- > Steve > www.lucidworks.com > > > On Jun 20, 2018, at 5:30 PM, David Smiley <[email protected]> wrote: > > > > +1 Sounds good Steve; thanks for working with Gavin and Infra to improve > > our workflow. > > > > It'd be nice if, after cancelling a patch review, I could re-enable it. It > > appears the only way to do this is to re-attach the patch? Any way it's a > > minor issue. I just did some fooling around on INFRATEST to try. > > > > ~ David > > > > > > On Tue, Jun 19, 2018 at 1:53 PM Steve Rowe <[email protected]> wrote: > > The LUCENE and SOLR JIRA projects’ workflow was changed to support > > automatic patch validation via Apache Yetus[1], but there have been > > objections to the new workflow and button labels - see INFRA-16094[2]. > > > > Under INFRA-16094, Gavin McDonald has produced a new workflow for > > LUCENE/SOLR that addresses the issues raised there. Below I’ll summarize > > the changes to the workflow, which is now demo'd on the JIRA project named > > INFRATEST1[3]. > > > > This email is a request for review of the proposed workflow changes prior > > to putting them in place. FYI, Gavin has offered to change other aspects > > of the LUCENE/SOLR workflow, so if you have any pet peeves, now is the time > > to get them addressed (but see my “separate issue” under the workflow > > changes summary below). > > > > Please post comments either on this thread or on INFRA-16094 (I’ll update > > there if you comment on this thread and it makes sense to notify Infra). > > > > ----- > > Summary of the workflow changes: > > > > 1. The “Submit Patch” button will be relabeled “Attach Patch”, and will > > bring up the dialog to attach a patch, with a simultaneous comment (rather > > than just changing the issue status). This button will remain visible > > regardless of issue status, so that it can be used to attach more patches. > > > > 2. In the “Attach Patch” dialog, there will be a checkbox labeled “Enable > > Automatic Patch Validation”, which will be checked by default. If checked, > > the issue’s status will transition to “Patch Available” (which signals > > Yetus to perform automatic patch validation); if not checked, the patch > > will be attached but no status transition will occur. NOTE: Gavin is still > > working on adding this checkbox, so it’s not demo’d on INFRATEST1 issues > > yet, but he says it’s doable and that he’ll work on it tomorrow, Australia > > time. > > > > 3. When in “Patch Available” status, a button labeled “Cancel Patch Review” > > will be visible; clicking on it will transition the issue status to “Open”, > > thus disabling automatic patch review. > > > > 4. The “Start Progress”/“Stop Progress”/“In Progress” aspects of the > > workflow have been removed, because if they remain, JIRA creates a > > “Workflow” menu and puts the “Attach Patch” button under it, which kind of > > defeats its purpose: an obvious way to submit contributions. I asked Gavin > > to remove the “Progress” related aspects of the workflow because I don’t > > think they’re being used except on a limited ad-hoc basis, not part of a > > conventional workflow. > > ----- > > > > Separate issue: on the thread where Cassandra moved the “Enviroment” field > > below “Description” on the Create JIRA dialog[4], David Smiley wrote[5]: > > > > > ok and these Lucene Fields, two checkboxes, New and Patch Available... I > > > just don't think we really use this but I should raise this separately. > > > > I think we should remove these. In a chat on Infra Hipchat, Gavin offered > > to do this, but since the Lucene PMC has control of this (as part of > > “screen configuration”, which is separate from “workflow” configuration), I > > told him we would tackle it ourselves. > > > > [1] Enable Yetus for LUCENE/SOLR: > > https://issues.apache.org/jira/browse/INFRA-15213 > > [2] Modify LUCENE/SOLR Yetus-enabling workflow: > > https://issues.apache.org/jira/browse/INFRA-16094 > > [3] Demo of proposed LUCENE/SOLR workflow: > > https://issues.apache.org/jira/projects/INFRATEST1 > > [4] Cassandra fixes Create JIRA dialog: > > https://lists.apache.org/thread.html/0efebe2fb08c7584421422d6005401a987a2b54bf604ae317b6e102f@%3Cdev.lucene.apache.org%3E > > [5] David Smiley says "Lucene fields” are unused: > > https://lists.apache.org/thread.html/a17bd3b5797c12903d3c6bacb348e8b4325c59609765964527412ba4@%3Cdev.lucene.apache.org%3E > > > > -- > > Steve > > www.lucidworks.com > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > -- > > Lucene/Solr Search Committer, Consultant, Developer, Author, Speaker > > LinkedIn: http://linkedin.com/in/davidwsmiley | Book: > > http://www.solrenterprisesearchserver.com > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
