You are just renaming the step where nobody is supposed to make changes any more (from positive review to merging). It doesn't solve anything. Eventually a CI script will have to be written, and it'll automatically switch from positive review to merging.
We could add a mandatory 2-week cooling off period while I'm not merging positively-reviewed tickets, if thats what you want. But I'd find that annoying. On Thursday, April 16, 2015 at 1:33:51 AM UTC+2, Nils Bruin wrote: > > On Wednesday, April 15, 2015 at 2:37:02 PM UTC-7, Volker Braun wrote: >> >> Even then, the correct workflow is to not change tickets after setting >> them to positive review. Having the release manager chase after branches as >> people keep adding stuff after review is not a sustainable workflow. >> > > I completely concur. The whole meaning of "positive review" goes out of > the window if one still changes the branch on the ticket. Obviously, before > someone can change the branch on the ticket, they should set its status to > "needs work" (or at least some other status. Perhaps the act of changing > the branch should automatically remove "positive review" status? However, > even with doing that, there is still a race condition possible. I'm not > quite sure how likely this one is and hence whether we need to protect > against it. The consequences are just an unmerged further change, which > should not be a huge disaster: > > 0. Ticket is at "positive review" > 1. Release Manager (RM) starts testing/merging process > 2. Ticket gets set to "needs work" by author > 3. Branch gets updated by author > 4. Referee (sits next to author, perhaps) agrees with changes and set > ticket to "positive review" (he may have witnessed testing before step 2 > already) > 5. RM finishes testing/merging and if he/she checks at all, would find the > ticket at "positive review" as expected and now moves status to "closed", > but has only merged the old branch. > > The obvious solution is to change step 1 to: > 1'. RM sets status of ticket to "merging" and starts testing/merging > process (this change in status should be made by the same script that > starts the testing merging process, in which case it would not add to the > workload of the RM, only to a more elaborate ticket history) > > I don't think the extra status would even need to be protected so that the > RM can only change it to something else: the flag should be enough. > > > -- You received this message because you are subscribed to the Google Groups "sage-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+unsubscr...@googlegroups.com. To post to this group, send email to sage-devel@googlegroups.com. Visit this group at http://groups.google.com/group/sage-devel. For more options, visit https://groups.google.com/d/optout.
