You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Cody Maloney <co...@mesosphere.io> on 2014/10/16 23:37:12 UTC
Review Request 26827: MESOS-1881: Reviewbot should not apply reviews
that are submitted.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26827/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Bugs: MESOS-1881
https://issues.apache.org/jira/browse/MESOS-1881
Repository: mesos-git
Description
-------
If the status is submitted, don't actually apply the review.
We still recurse throuhg it for dependent reviews so we get any open reviews in a chain (although open -> submitted -> open should be uncommon in our workflow).
Diffs
-----
support/verify-reviews.py 9408747995db4b78ac6edd74995a05ab0355f1df
Diff: https://reviews.apache.org/r/26827/diff/
Testing
-------
Ran locally (With result posting + building disabled), adding a print statement when a review in the dependency set was skipped because it was submitted:
Sample output:
```
Checking if review: 26071 needs verification
Skipping blocking review 26071
Checking if review: 26727 needs verification
Skipping blocking review 26727
Checking if review: 26796 needs verification
Skipping blocking review 26796
Checking if review: 26797 needs verification
Latest diff timestamp: 2014-10-16 19:16:26
Latest dependency change timestamp: 2014-10-16 19:23:07
Verifying review 26797
Dependent review: https://reviews.apache.org/api/review-requests/26796/
Dependent review: https://reviews.apache.org/api/review-requests/26727/
Dependent review: https://reviews.apache.org/api/review-requests/26071/
Dependent review: https://reviews.apache.org/api/review-requests/26789/
Dependent review: https://reviews.apache.org/api/review-requests/26674/
Skipping submitted review 26674
Skipping submitted review 26789
Dependent review: https://reviews.apache.org/api/review-requests/26791/
Skipping submitted review 26791
Applying review 26071
./support/apply-review.sh -n -r 26071
Applying review 26727
./support/apply-review.sh -n -r 26727
Posting review: Bad patch!
Reviews applied: [26674, 26789, 26791, 26071, 26727]
Failed command: ./support/apply-review.sh -n -r 26727
Error:
--2014-10-16 14:34:43-- https://reviews.apache.org/r/26727/diff/raw/
Resolving reviews.apache.org... 140.211.11.74
Connecting to reviews.apache.org|140.211.11.74|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 1574 (1.5K) [text/x-patch]
Saving to: '26727.patch'
0K . 100% 34.9M=0s
2014-10-16 14:34:43 (34.9 MB/s) - '26727.patch' saved [1574/1574]
error: patch failed: src/module/manager.hpp:89
error: src/module/manager.hpp: patch does not apply
Failed to apply patch
```
Thanks,
Cody Maloney
Re: Review Request 26827: MESOS-1881: Reviewbot should not apply
reviews that are submitted.
Posted by Ben Mahler <be...@gmail.com>.
> On Oct. 16, 2014, 10:54 p.m., Ben Mahler wrote:
> > Thanks! I'll get this committed shortly.
FYI I cleaned up the commit message to not reference reviewbot, since it's not a concept in the source:
commit f6aa4b04d2e669e1e6cbcaf8fb708ab1cd6410eb
Author: Cody Maloney <co...@mesosphere.io>
Date: Thu Oct 16 15:54:28 2014 -0700
Fixed verify-reviews.py to not apply submitted reviews.
Review: https://reviews.apache.org/r/26827
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26827/#review57026
-----------------------------------------------------------
On Oct. 16, 2014, 9:37 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26827/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 9:37 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1881
> https://issues.apache.org/jira/browse/MESOS-1881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> If the status is submitted, don't actually apply the review.
>
> We still recurse throuhg it for dependent reviews so we get any open reviews in a chain (although open -> submitted -> open should be uncommon in our workflow).
>
>
> Diffs
> -----
>
> support/verify-reviews.py 9408747995db4b78ac6edd74995a05ab0355f1df
>
> Diff: https://reviews.apache.org/r/26827/diff/
>
>
> Testing
> -------
>
> Ran locally (With result posting + building disabled), adding a print statement when a review in the dependency set was skipped because it was submitted:
>
> Sample output:
> ```
> Checking if review: 26071 needs verification
> Skipping blocking review 26071
> Checking if review: 26727 needs verification
> Skipping blocking review 26727
> Checking if review: 26796 needs verification
> Skipping blocking review 26796
> Checking if review: 26797 needs verification
> Latest diff timestamp: 2014-10-16 19:16:26
> Latest dependency change timestamp: 2014-10-16 19:23:07
> Verifying review 26797
> Dependent review: https://reviews.apache.org/api/review-requests/26796/
> Dependent review: https://reviews.apache.org/api/review-requests/26727/
> Dependent review: https://reviews.apache.org/api/review-requests/26071/
> Dependent review: https://reviews.apache.org/api/review-requests/26789/
> Dependent review: https://reviews.apache.org/api/review-requests/26674/
> Skipping submitted review 26674
> Skipping submitted review 26789
> Dependent review: https://reviews.apache.org/api/review-requests/26791/
> Skipping submitted review 26791
> Applying review 26071
> ./support/apply-review.sh -n -r 26071
> Applying review 26727
> ./support/apply-review.sh -n -r 26727
> Posting review: Bad patch!
>
> Reviews applied: [26674, 26789, 26791, 26071, 26727]
>
> Failed command: ./support/apply-review.sh -n -r 26727
>
> Error:
> --2014-10-16 14:34:43-- https://reviews.apache.org/r/26727/diff/raw/
> Resolving reviews.apache.org... 140.211.11.74
> Connecting to reviews.apache.org|140.211.11.74|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 1574 (1.5K) [text/x-patch]
> Saving to: '26727.patch'
>
> 0K . 100% 34.9M=0s
>
> 2014-10-16 14:34:43 (34.9 MB/s) - '26727.patch' saved [1574/1574]
>
> error: patch failed: src/module/manager.hpp:89
> error: src/module/manager.hpp: patch does not apply
> Failed to apply patch
>
> ```
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26827: MESOS-1881: Reviewbot should not apply
reviews that are submitted.
Posted by Cody Maloney <co...@mesosphere.io>.
> On Oct. 16, 2014, 10:54 p.m., Ben Mahler wrote:
> > support/verify-reviews.py, lines 84-87
> > <https://reviews.apache.org/r/26827/diff/1/?file=723390#file723390line84>
> >
> > We probably shouldn't still be saying "Now apply this review and then say skip it.
> >
> > I'll adjust this for you since it's pretty trivial. :)
> >
> > Remember to end comments with periods and avoid squashing blocks together without whitespace (line 83 and 84).
Thanks! I'm working on it for hte periods. Takes some practice.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26827/#review57026
-----------------------------------------------------------
On Oct. 16, 2014, 9:37 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26827/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 9:37 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1881
> https://issues.apache.org/jira/browse/MESOS-1881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> If the status is submitted, don't actually apply the review.
>
> We still recurse throuhg it for dependent reviews so we get any open reviews in a chain (although open -> submitted -> open should be uncommon in our workflow).
>
>
> Diffs
> -----
>
> support/verify-reviews.py 9408747995db4b78ac6edd74995a05ab0355f1df
>
> Diff: https://reviews.apache.org/r/26827/diff/
>
>
> Testing
> -------
>
> Ran locally (With result posting + building disabled), adding a print statement when a review in the dependency set was skipped because it was submitted:
>
> Sample output:
> ```
> Checking if review: 26071 needs verification
> Skipping blocking review 26071
> Checking if review: 26727 needs verification
> Skipping blocking review 26727
> Checking if review: 26796 needs verification
> Skipping blocking review 26796
> Checking if review: 26797 needs verification
> Latest diff timestamp: 2014-10-16 19:16:26
> Latest dependency change timestamp: 2014-10-16 19:23:07
> Verifying review 26797
> Dependent review: https://reviews.apache.org/api/review-requests/26796/
> Dependent review: https://reviews.apache.org/api/review-requests/26727/
> Dependent review: https://reviews.apache.org/api/review-requests/26071/
> Dependent review: https://reviews.apache.org/api/review-requests/26789/
> Dependent review: https://reviews.apache.org/api/review-requests/26674/
> Skipping submitted review 26674
> Skipping submitted review 26789
> Dependent review: https://reviews.apache.org/api/review-requests/26791/
> Skipping submitted review 26791
> Applying review 26071
> ./support/apply-review.sh -n -r 26071
> Applying review 26727
> ./support/apply-review.sh -n -r 26727
> Posting review: Bad patch!
>
> Reviews applied: [26674, 26789, 26791, 26071, 26727]
>
> Failed command: ./support/apply-review.sh -n -r 26727
>
> Error:
> --2014-10-16 14:34:43-- https://reviews.apache.org/r/26727/diff/raw/
> Resolving reviews.apache.org... 140.211.11.74
> Connecting to reviews.apache.org|140.211.11.74|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 1574 (1.5K) [text/x-patch]
> Saving to: '26727.patch'
>
> 0K . 100% 34.9M=0s
>
> 2014-10-16 14:34:43 (34.9 MB/s) - '26727.patch' saved [1574/1574]
>
> error: patch failed: src/module/manager.hpp:89
> error: src/module/manager.hpp: patch does not apply
> Failed to apply patch
>
> ```
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26827: MESOS-1881: Reviewbot should not apply
reviews that are submitted.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26827/#review57026
-----------------------------------------------------------
Ship it!
Thanks! I'll get this committed shortly.
support/verify-reviews.py
<https://reviews.apache.org/r/26827/#comment97487>
We probably shouldn't still be saying "Now apply this review and then say skip it.
I'll adjust this for you since it's pretty trivial. :)
Remember to end comments with periods and avoid squashing blocks together without whitespace (line 83 and 84).
- Ben Mahler
On Oct. 16, 2014, 9:37 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26827/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 9:37 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1881
> https://issues.apache.org/jira/browse/MESOS-1881
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> If the status is submitted, don't actually apply the review.
>
> We still recurse throuhg it for dependent reviews so we get any open reviews in a chain (although open -> submitted -> open should be uncommon in our workflow).
>
>
> Diffs
> -----
>
> support/verify-reviews.py 9408747995db4b78ac6edd74995a05ab0355f1df
>
> Diff: https://reviews.apache.org/r/26827/diff/
>
>
> Testing
> -------
>
> Ran locally (With result posting + building disabled), adding a print statement when a review in the dependency set was skipped because it was submitted:
>
> Sample output:
> ```
> Checking if review: 26071 needs verification
> Skipping blocking review 26071
> Checking if review: 26727 needs verification
> Skipping blocking review 26727
> Checking if review: 26796 needs verification
> Skipping blocking review 26796
> Checking if review: 26797 needs verification
> Latest diff timestamp: 2014-10-16 19:16:26
> Latest dependency change timestamp: 2014-10-16 19:23:07
> Verifying review 26797
> Dependent review: https://reviews.apache.org/api/review-requests/26796/
> Dependent review: https://reviews.apache.org/api/review-requests/26727/
> Dependent review: https://reviews.apache.org/api/review-requests/26071/
> Dependent review: https://reviews.apache.org/api/review-requests/26789/
> Dependent review: https://reviews.apache.org/api/review-requests/26674/
> Skipping submitted review 26674
> Skipping submitted review 26789
> Dependent review: https://reviews.apache.org/api/review-requests/26791/
> Skipping submitted review 26791
> Applying review 26071
> ./support/apply-review.sh -n -r 26071
> Applying review 26727
> ./support/apply-review.sh -n -r 26727
> Posting review: Bad patch!
>
> Reviews applied: [26674, 26789, 26791, 26071, 26727]
>
> Failed command: ./support/apply-review.sh -n -r 26727
>
> Error:
> --2014-10-16 14:34:43-- https://reviews.apache.org/r/26727/diff/raw/
> Resolving reviews.apache.org... 140.211.11.74
> Connecting to reviews.apache.org|140.211.11.74|:443... connected.
> HTTP request sent, awaiting response... 200 OK
> Length: 1574 (1.5K) [text/x-patch]
> Saving to: '26727.patch'
>
> 0K . 100% 34.9M=0s
>
> 2014-10-16 14:34:43 (34.9 MB/s) - '26727.patch' saved [1574/1574]
>
> error: patch failed: src/module/manager.hpp:89
> error: src/module/manager.hpp: patch does not apply
> Failed to apply patch
>
> ```
>
>
> Thanks,
>
> Cody Maloney
>
>