You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/03/03 06:08:05 UTC

Review Request 18676: Added a verify-reviews.py script that can run via CI to verify ReviewBoard reviews.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/
-----------------------------------------------------------

Review request for mesos, Benjamin Hindman and Ben Mahler.


Bugs: MESOS-1052
    https://issues.apache.org/jira/browse/MESOS-1052


Repository: mesos-git


Description
-------

Created a RB account for the ReviewBot so that it can post reviews.


Diffs
-----

  support/verify-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/18676/diff/


Testing
-------

Verified (:)) by slightly modifying the script and testing on the following review.

https://reviews.apache.org/r/18674/


Thanks,

Vinod Kone


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify Review Board reviews.

Posted by Mesos ReviewBot <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/#review36156
-----------------------------------------------------------


Bad patch!

Reviews applied: []

Failed command: git apply --index 18676.patch

Error:
 error: support/verify-reviews.py: already exists in index


- Mesos ReviewBot


On March 4, 2014, 6 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18676/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 6 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1052
>     https://issues.apache.org/jira/browse/MESOS-1052
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created a RB account for the ReviewBot so that it can post reviews.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18676/diff/
> 
> 
> Testing
> -------
> 
> Verified (:)) by slightly modifying the script and testing on the following review.
> 
> https://reviews.apache.org/r/18674/
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify Review Board reviews.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/#review36131
-----------------------------------------------------------



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment67053>

    Sweet PEP 8 my friend.



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment67055>

    How about 'atexit' delete the patch?


- Benjamin Hindman


On March 4, 2014, 6 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18676/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 6 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1052
>     https://issues.apache.org/jira/browse/MESOS-1052
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created a RB account for the ReviewBot so that it can post reviews.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18676/diff/
> 
> 
> Testing
> -------
> 
> Verified (:)) by slightly modifying the script and testing on the following review.
> 
> https://reviews.apache.org/r/18674/
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify Review Board reviews.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/#review36158
-----------------------------------------------------------

Ship it!


Ship It!

- Dominic Hamon


On March 4, 2014, 12:20 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18676/
> -----------------------------------------------------------
> 
> (Updated March 4, 2014, 12:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1052
>     https://issues.apache.org/jira/browse/MESOS-1052
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created a RB account for the ReviewBot so that it can post reviews.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18676/diff/
> 
> 
> Testing
> -------
> 
> Verified (:)) by slightly modifying the script and testing on the following review.
> 
> https://reviews.apache.org/r/18674/
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify Review Board reviews.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/
-----------------------------------------------------------

(Updated March 4, 2014, 8:20 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

benh's comments. also addressed the multiple dependents issue. PTAL.


Bugs: MESOS-1052
    https://issues.apache.org/jira/browse/MESOS-1052


Repository: mesos-git


Description
-------

Created a RB account for the ReviewBot so that it can post reviews.


Diffs (updated)
-----

  support/verify-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/18676/diff/


Testing
-------

Verified (:)) by slightly modifying the script and testing on the following review.

https://reviews.apache.org/r/18674/


Thanks,

Vinod Kone


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify Review Board reviews.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/
-----------------------------------------------------------

(Updated March 4, 2014, 6 p.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

made the query params also configurable via command line to selectively turn this on specific reviews as we test this feature.


Bugs: MESOS-1052
    https://issues.apache.org/jira/browse/MESOS-1052


Repository: mesos-git


Description
-------

Created a RB account for the ReviewBot so that it can post reviews.


Diffs (updated)
-----

  support/verify-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/18676/diff/


Testing
-------

Verified (:)) by slightly modifying the script and testing on the following review.

https://reviews.apache.org/r/18674/


Thanks,

Vinod Kone


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify Review Board reviews.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/
-----------------------------------------------------------

(Updated March 4, 2014, 8:36 a.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

benh's and dominic's.

also, changed the semantics to skip reviews that block others instead of reviews that depends on others.


Summary (updated)
-----------------

Added a verify-reviews.py script that can run via CI to verify Review Board reviews.


Bugs: MESOS-1052
    https://issues.apache.org/jira/browse/MESOS-1052


Repository: mesos-git


Description
-------

Created a RB account for the ReviewBot so that it can post reviews.


Diffs (updated)
-----

  support/verify-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/18676/diff/


Testing
-------

Verified (:)) by slightly modifying the script and testing on the following review.

https://reviews.apache.org/r/18674/


Thanks,

Vinod Kone


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify ReviewBoard reviews.

Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/#review35974
-----------------------------------------------------------



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66764>

    indent +1 for this method



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66765>

    Might be worth logging these cases for later debugging.


- Dominic Hamon


On March 2, 2014, 11:33 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18676/
> -----------------------------------------------------------
> 
> (Updated March 2, 2014, 11:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1052
>     https://issues.apache.org/jira/browse/MESOS-1052
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created a RB account for the ReviewBot so that it can post reviews.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18676/diff/
> 
> 
> Testing
> -------
> 
> Verified (:)) by slightly modifying the script and testing on the following review.
> 
> https://reviews.apache.org/r/18674/
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify ReviewBoard reviews.

Posted by Vinod Kone <vi...@gmail.com>.

> On March 4, 2014, 1:33 a.m., Benjamin Hindman wrote:
> > support/verify-reviews.py, lines 32-34
> > <https://reviews.apache.org/r/18676/diff/2/?file=507878#file507878line32>
> >
> >     Could we always delete the patch file (like apply-review.sh) in case someone uses this locally?

"git clean -fd" at the end of verify_review should clean the patch up.


> On March 4, 2014, 1:33 a.m., Benjamin Hindman wrote:
> > support/verify-reviews.py, line 43
> > <https://reviews.apache.org/r/18676/diff/2/?file=507878#file507878line43>
> >
> >     Why not s/url/href/ instead?

the json doesn't contain url unfortunately.


> On March 4, 2014, 1:33 a.m., Benjamin Hindman wrote:
> > support/verify-reviews.py, line 79
> > <https://reviews.apache.org/r/18676/diff/2/?file=507878#file507878line79>
> >
> >     Also s/url/href/ here?

json doesn't contain url.


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/#review35986
-----------------------------------------------------------


On March 3, 2014, 7:33 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18676/
> -----------------------------------------------------------
> 
> (Updated March 3, 2014, 7:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1052
>     https://issues.apache.org/jira/browse/MESOS-1052
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created a RB account for the ReviewBot so that it can post reviews.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18676/diff/
> 
> 
> Testing
> -------
> 
> Verified (:)) by slightly modifying the script and testing on the following review.
> 
> https://reviews.apache.org/r/18674/
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify ReviewBoard reviews.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/#review35986
-----------------------------------------------------------

Ship it!


Awesome, looking forward to getting this running!


support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66773>

    s/Reviewboard/Review Board/



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66933>

    What about getting the password from the command line? Then the Jenkins bot can be configured with the password but what the "public" sees in the description box (or what ever it's called) would just be #####.



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66774>

    We should really be moving to using the PEP 8 style guide, which is +4 indent.



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66935>

    Spaces.



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66934>

    Could we always delete the patch file (like apply-review.sh) in case someone uses this locally?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66936>

    Why not s/url/href/ instead?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66937>

    What about defining the 'api' function above this usage?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66938>

    .



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66939>

    Move 'post_review' above too?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66940>

    Also s/url/href/ here?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66945>

    Newline before this?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66946>

    Spaces.



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66947>

    Is 'not' here really better than '= 0'?



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66948>

    Spaces (equals and after comma).



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66950>

    Spaces.



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66951>

    Spaces (equals and after comma).



support/verify-reviews.py
<https://reviews.apache.org/r/18676/#comment66942>

    s/mesos/Mesos/


- Benjamin Hindman


On March 3, 2014, 7:33 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18676/
> -----------------------------------------------------------
> 
> (Updated March 3, 2014, 7:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Ben Mahler.
> 
> 
> Bugs: MESOS-1052
>     https://issues.apache.org/jira/browse/MESOS-1052
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Created a RB account for the ReviewBot so that it can post reviews.
> 
> 
> Diffs
> -----
> 
>   support/verify-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18676/diff/
> 
> 
> Testing
> -------
> 
> Verified (:)) by slightly modifying the script and testing on the following review.
> 
> https://reviews.apache.org/r/18674/
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>


Re: Review Request 18676: Added a verify-reviews.py script that can run via CI to verify ReviewBoard reviews.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18676/
-----------------------------------------------------------

(Updated March 3, 2014, 7:33 a.m.)


Review request for mesos, Benjamin Hindman and Ben Mahler.


Changes
-------

minor fixes.


Bugs: MESOS-1052
    https://issues.apache.org/jira/browse/MESOS-1052


Repository: mesos-git


Description
-------

Created a RB account for the ReviewBot so that it can post reviews.


Diffs (updated)
-----

  support/verify-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/18676/diff/


Testing
-------

Verified (:)) by slightly modifying the script and testing on the following review.

https://reviews.apache.org/r/18674/


Thanks,

Vinod Kone