----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70000/#review212917 -----------------------------------------------------------
support/verify-reviews.py Line 182 (original), 188 (patched) <https://reviews.apache.org/r/70000/#comment298793> hmm. i don't think this is the right place to catch this exception. what if we don't get into this for loop at all if it's a single review chain? i think it should be in #197. - Vinod Kone On Feb. 19, 2019, 4:44 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70000/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2019, 4:44 p.m.) > > > Review request for mesos, Till Toenshoff and Vinod Kone. > > > Bugs: MESOS-9582 > https://issues.apache.org/jira/browse/MESOS-9582 > > > Repository: mesos > > > Description > ------- > > Previously `support/verify-reviews.py` would abort prematurely > whenever a review a patch could not be applied. This was due to > the `shell` function used to call `support/apply-reviews.py` invoking > `exit(1)` on the first error which stopped iteration over all > outstanding reviews. > > In this patch that explicit call to `exit` is removed, and instead we > let a possible `subprocess.CalledProcessError` propagate up for it to > be handled at a higher level. Currently this will post a note on the > review in question to (1) notify the submitter, and (2) prevent the > review from being checked again. > > With the changes here the script will not stop verify reviews in such > cases. > > > Diffs > ----- > > support/verify-reviews.py a88a91f8ecb3794846d2d5eddee27ada770d55b9 > > > Diff: https://reviews.apache.org/r/70000/diff/3/ > > > Testing > ------- > > Ran the script (with a dummy user and password) on today's reviewboard state. > The script attempted to post a review on the last patch in the chain instead > of aborting (see the `TODO` in the code on why we weren't able to diagnose > the issue in the faulty patch with the current implementation). > > > Thanks, > > Benjamin Bannier > >
