----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70060/#review213333 -----------------------------------------------------------
Fix it, then Ship it! support/verify-reviews.py Lines 51 (patched) <https://reviews.apache.org/r/70060/#comment299283> Would could introduce a small function to perform the decoding instead of introducing this global, but very specific constant, e.g., def parse_reviewtime(timestamp): return datetime.strptime(timestamp, "%Y-%m-%dT%H:%M:%SZ") support/verify-reviews.py Lines 307 (patched) <https://reviews.apache.org/r/70060/#comment299286> Not a strong sentiment, but this function could also be called e.g., `changed_after` or similar since it exclusively deals with timestamps. support/verify-reviews.py Lines 308 (patched) <https://reviews.apache.org/r/70060/#comment299281> `Returns whether this review request chain needs to be verified.` support/verify-reviews.py Lines 310 (patched) <https://reviews.apache.org/r/70060/#comment299280> The references to `USER` seem leaky and unnecessary; could we remove them? Here and below. support/verify-reviews.py Line 307 (original), 349 (patched) <https://reviews.apache.org/r/70060/#comment299282> `s/True/whether/` support/verify-reviews.py Line 330 (original), 359 (patched) <https://reviews.apache.org/r/70060/#comment299285> Not yours, but it seems this should page through all open reviews. support/verify-reviews.py Line 339 (original), 368 (patched) <https://reviews.apache.org/r/70060/#comment299284> Nit: Let's move the comment above the conditional and expand it, e.g., # We verify the review if there was no previous review. - Benjamin Bannier On Feb. 28, 2019, 7:53 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70060/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2019, 7:53 p.m.) > > > Review request for mesos, Benjamin Bannier and Till Toenshoff. > > > Bugs: MESOS-4599 > https://issues.apache.org/jira/browse/MESOS-4599 > > > Repository: mesos > > > Description > ------- > > If any of the dependent reviews has an updated diff or dependency, it > now triggers the ReviewBot. Previously only updates to the tail > review in the chain triggered the ReviewBot. > > > Diffs > ----- > > support/verify-reviews.py f03869abd74e920283bbb2acc9c71f5f0f30e554 > > > Diff: https://reviews.apache.org/r/70060/diff/2/ > > > Testing > ------- > > ? mesos git:(vinod/reviewbot_recursive_diff_time) ? > ./support/verify-reviews.py -u mesos-review -p foo --skip-verify > Checking if review: 65835 needs verification > Skipping blocking review 65835 > Checking if review: 65836 needs verification > Latest review timestamp: 2018-02-28 14:43:21 > Latest diff timestamp: 2018-02-28 13:55:43 > Dependent review: https://reviews.apache.org/api/review-requests/65835/ > Latest diff timestamp: 2018-02-28 13:55:32 > Dependent review: https://reviews.apache.org/api/review-requests/65834/ > Latest diff timestamp: 2018-02-28 13:55:22 > Checking if review: 65820 needs verification > Skipping blocking review 65820 > Checking if review: 65821 needs verification > Skipping blocking review 65821 > Checking if review: 65847 needs verification > Latest review timestamp: 2018-03-01 02:54:04 > Latest diff timestamp: 2018-02-28 20:22:13 > Dependent review: https://reviews.apache.org/api/review-requests/65845/ > Latest diff timestamp: 2018-02-28 19:57:14 > Dependent review: https://reviews.apache.org/api/review-requests/65844/ > Latest diff timestamp: 2018-02-28 19:57:08 > Dependent review: https://reviews.apache.org/api/review-requests/65821/ > Latest diff timestamp: 2018-02-28 19:57:02 > Dependent review: https://reviews.apache.org/api/review-requests/65820/ > ... > ... > > > Thanks, > > Vinod Kone > >
