We can definitely make this a per-branch (i.e., per-RM) decision. It's just a little PHP code -- no problem.
Given timezone differences, give Gilles and me another day or three to get this all sorted out. So it'll be: - pushing commits will remove "reviewed" and "rm-approval" labels, and will add "pushed-back" if either of those labels were actually removed - adding "reviewed" or "rm-approval" will remove the "pushed-back" label > On Feb 5, 2015, at 11:32 AM, Ralph Castain <r...@open-mpi.org> wrote: > > I agree that others should be responsible for the review. However, just > saying “well they did a sloppy job” doesn’t resolve the problem. It has > happened on more than one occasion, so it is something that tends to slip by. > > If the community doesn’t want to remove the approval, then I can live with it > - I’ll just mentally make a note to recheck whether the scope has changed. > More burden, but that seems to be the way we are going. > > >> On Feb 5, 2015, at 8:25 AM, George Bosilca <bosi...@icl.utk.edu> wrote: >> >> The RM should not be expected to read and accept the code itself, but his >> role should be limited to accepting the idea behind the patch and making >> sure it is compatible with the rules in place. As such, removing the >> RM-approval mark is not yielding any benefits. >> >> Moreover, based on the ideas proposed above (any modification removes the >> reviewed marker), if the scenario depicted by Ralph happens again I would >> argue that the reviewers would have done a sloppy job at allowing the patch >> to drift from it's original specification (the one approved by the RM). >> >> George. >> >> >> On Thu, Feb 5, 2015 at 10:33 AM, Ralph Castain <r...@open-mpi.org> wrote: >> >>> On Feb 5, 2015, at 7:17 AM, Howard Pritchard <hpprit...@gmail.com> wrote: >>> >>> Hi Jeff >>> >>> Gilles ideas are great. >>> >>> I agree with your RM stamp of approval policy. No removal of rm approved in >>> the event of subsequent commits. >>> >>> >> >> I disagree, so perhaps that should be something settable by the RM for a >> given release? I’ve been burned before where I approved something, then >> someone added a bunch of unrelated code that caused a significant change >> (i.e., modifying prior behavior) without warning. Result: users yelling, >> chasing it down, reverting half of the commit, and then re-releasing. >> >> I’d rather not have that happen again. >> >>> Howard >>> >>> On Feb 5, 2015 5:04 AM, "Jeff Squyres (jsquyres)" <jsquy...@cisco.com> >>> wrote: >>> Gilles came up with a cool idea for the OMPIBot (see below). We can do >>> this idea, but I want to make sure that everyone is ok with it first. >>> >>> Consider this scenario: >>> >>> 1. You create a PR >>> 2. Over time, it gets reviewed, and then RM approved (i.e., the "reviewed" >>> and "rm-approved" labels are added). >>> 3. *** But then new commits are pushed to the PR. >>> >>> --> Technically, it should really be reviewed again before it is merged. >>> Here's what Gilles came up with: >>> >>> 4. The OMPIBot can tell when new commits are pushed, and can: >>> 4a) remove the "reviewed" label, and >>> 4b) add the "pushed-back" label >>> 5. Further, whenever someone adds the "reviewed" label, OMPIBot can >>> automatically remove the "pushed-back" label. >>> >>> I.e., when you add commits to an already-reviewed PR, you lose "reviewed", >>> but you get a positive signal in the form of the "pushed-back" label, >>> reminding you that you need to get it reviewed again. And when someone >>> reviews it, it automatically removes the "pushed-back" label. >>> >>> Finally, here's a question to the RM: if someone pushes new commits to a PR >>> after it has been rm-approved, do you want the rm-approved label removed? >>> My gut feeling is "no" -- it stays approved. >>> >>> Thoughts? >>> >>> >>> >>> On Feb 4, 2015, at 2:26 PM, Howard Pritchard <hpprit...@gmail.com> wrote: >>> > >>> > +1 >>> > great stuff >>> > >>> > 2015-02-04 5:55 GMT-07:00 Jeff Squyres (jsquyres) <jsquy...@cisco.com>: >>> > OMPI devs -- >>> > >>> > Per lots of previous discussions, you all know that you can't assign >>> > labels, milestones, or users to issues/pull requests on the ompi-release >>> > repo. >>> > >>> > Gilles has written a Github bot that will allow you to do these things by >>> > inserting special tokens in the text of issues/pull requests/comments. >>> > Here's an example: >>> > >>> > This PR fixes problem XYZ. >>> > >>> > label:bug >>> > label:enhancement >>> > milestone:v1.8.5 >>> > assign:@jsquyres >>> > >>> > *** PLEASE GO TRY IT on the sandbox ompi-release-bot repo. >>> > >>> > Here's a fuller explanation of what OMPIBot does, and links to where you >>> > can try it out: >>> > >>> > https://github.com/open-mpi/ompi-release-bot/wiki >>> > >>> > Once we get enough people to try it/fix any bugs/etc., we'll deploy it on >>> > the ompi-release repo. >>> > >>> > -- >>> > Jeff Squyres >>> > jsquy...@cisco.com >>> > For corporate legal information go to: >>> > http://www.cisco.com/web/about/doing_business/legal/cri/ >>> > >>> > _______________________________________________ >>> > devel mailing list >>> > de...@open-mpi.org >>> > Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel >>> > Link to this post: >>> > http://www.open-mpi.org/community/lists/devel/2015/02/16924.php >>> > >>> > _______________________________________________ >>> > devel mailing list >>> > de...@open-mpi.org >>> > Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel >>> > Link to this post: >>> > http://www.open-mpi.org/community/lists/devel/2015/02/16925.php >>> >>> >>> -- >>> Jeff Squyres >>> jsquy...@cisco.com >>> For corporate legal information go to: >>> http://www.cisco.com/web/about/doing_business/legal/cri/ >>> >>> _______________________________________________ >>> devel mailing list >>> de...@open-mpi.org >>> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel >>> Link to this post: >>> http://www.open-mpi.org/community/lists/devel/2015/02/16927.php >>> _______________________________________________ >>> devel mailing list >>> de...@open-mpi.org >>> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel >>> Link to this post: >>> http://www.open-mpi.org/community/lists/devel/2015/02/16935.php >> >> >> _______________________________________________ >> devel mailing list >> de...@open-mpi.org >> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel >> Link to this post: >> http://www.open-mpi.org/community/lists/devel/2015/02/16936.php >> >> _______________________________________________ >> devel mailing list >> de...@open-mpi.org >> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel >> Link to this post: >> http://www.open-mpi.org/community/lists/devel/2015/02/16937.php > > _______________________________________________ > devel mailing list > de...@open-mpi.org > Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel > Link to this post: > http://www.open-mpi.org/community/lists/devel/2015/02/16938.php -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/
