You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Dragos Schebesch via Review Board <no...@reviews.apache.org> on 2018/06/08 12:07:43 UTC

Review Request 67503: Added helper for fetching review id

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

Review request for mesos and Andrew Schwartzmeyer.


Repository: mesos


Description
-------

Added helper for fetching review id


Diffs (updated)
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/1/


Testing
-------


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added helper for fetching review id

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/#review205905
-----------------------------------------------------------



The commit message should be edited to finish with a period and the Testing Done part should be filled even if it's just to say that tests have been done later in the chain.


support/python3/get-review-ids.py
Lines 19 (patched)
<https://reviews.apache.org/r/67503/#comment288803>

    Add a newline here, it's either:
    
    ```
    """Hello"""
    ```
    
    or
    
    ```
    """
    Lorem ipsum dolor sit amet, consectetur adipiscing elit.
    Etiam lectus orci, sodales a est in, vulputate egestas turpis. 
    Quisque quis rhoncus elit.
    """
    ```
    
    Also, s/`This file is used to get the`/`This script gets`.



support/python3/get-review-ids.py
Lines 30 (patched)
<https://reviews.apache.org/r/67503/#comment288804>

    Missing period.



support/python3/get-review-ids.py
Lines 41 (patched)
<https://reviews.apache.org/r/67503/#comment288805>

    Add a newline after the `""""`.



support/python3/get-review-ids.py
Lines 42 (patched)
<https://reviews.apache.org/r/67503/#comment288806>

    Missing period.



support/python3/get-review-ids.py
Lines 49 (patched)
<https://reviews.apache.org/r/67503/#comment288807>

    What happens if `--out-file` is not used?


- Armand Grillet


On June 14, 2018, 11:13 p.m., Dragos Schebesch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67503/
> -----------------------------------------------------------
> 
> (Updated June 14, 2018, 11:13 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added helper for fetching review id
> 
> 
> Diffs
> -----
> 
>   support/python3/get-review-ids.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67503/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>


Re: Review Request 67503: Added support helper for fetching review ids.

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/#review206382
-----------------------------------------------------------


Fix it, then Ship it!




LGTM except the first line. FYI the commit header and message should contain no line that are more than 72 characters.


support/python3/get-review-ids.py
Lines 1 (patched)
<https://reviews.apache.org/r/67503/#comment290026>

    Should be `python3`.


- Armand Grillet


On July 19, 2018, 3:48 p.m., Dragos Schebesch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67503/
> -----------------------------------------------------------
> 
> (Updated July 19, 2018, 3:48 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added helper for fetching review id, which is needed to decouple this logic from verify-reviews.py. We need this for the windows build so we can trigger a downstream job for each review request on another server.
> 
> 
> Diffs
> -----
> 
>   support/python3/get-review-ids.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67503/diff/4/
> 
> 
> Testing
> -------
> 
> Sample run on this review request id:
> ```
> ./get-review-ids.py -r 67503
> Dependent review: https://reviews.apache.org/api/review-requests/67502/
> 67502
> 
> 67503
> ```
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>


Re: Review Request 67503: Added support helper for fetching review ids.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/#review207242
-----------------------------------------------------------


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Aug. 7, 2018, 10:36 a.m., Dragos Schebesch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67503/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2018, 10:36 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added support helper for fetching review ids.
> 
> 
> Diffs
> -----
> 
>   support/python3/get-review-ids.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67503/diff/5/
> 
> 
> Testing
> -------
> 
> Sample run on this review request id:
> ```
> ./get-review-ids.py -r 67503
> Dependent review: https://reviews.apache.org/api/review-requests/67502/
> 67502
> 
> 67503
> ```
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>


Re: Review Request 67503: Added support helper for fetching review ids.

Posted by Dragos Schebesch via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/
-----------------------------------------------------------

(Updated Aug. 7, 2018, 5:36 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.


Repository: mesos


Description (updated)
-------

Added support helper for fetching review ids.


Diffs (updated)
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/5/

Changes: https://reviews.apache.org/r/67503/diff/4-5/


Testing
-------

Sample run on this review request id:
```
./get-review-ids.py -r 67503
Dependent review: https://reviews.apache.org/api/review-requests/67502/
67502

67503
```


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added support helper for fetching review ids.

Posted by Dragos Schebesch via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/
-----------------------------------------------------------

(Updated July 19, 2018, 3:48 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.


Repository: mesos


Description (updated)
-------

Added helper for fetching review id, which is needed to decouple this logic from verify-reviews.py. We need this for the windows build so we can trigger a downstream job for each review request on another server.


Diffs
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/4/


Testing
-------

Sample run on this review request id:
```
./get-review-ids.py -r 67503
Dependent review: https://reviews.apache.org/api/review-requests/67502/
67502

67503
```


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added support helper for fetching review ids.

Posted by Dragos Schebesch via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/
-----------------------------------------------------------

(Updated July 18, 2018, 8:50 a.m.)


Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.


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

Added support helper for fetching review ids.


Repository: mesos


Description (updated)
-------

Added support helper for fetching review ids.


Diffs (updated)
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/4/

Changes: https://reviews.apache.org/r/67503/diff/3-4/


Testing (updated)
-------

Sample run on this review request id:
```
./get-review-ids.py -r 67503
Dependent review: https://reviews.apache.org/api/review-requests/67502/
67502

67503
```


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added helper for fetching review id.

Posted by Dragos Schebesch via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/
-----------------------------------------------------------

(Updated July 11, 2018, 11:54 a.m.)


Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.


Repository: mesos


Description
-------

Added helper for fetching review id.


Diffs
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/3/


Testing (updated)
-------

Tests have been done later in the chain.


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added helper for fetching review id.

Posted by Dragos Schebesch via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/
-----------------------------------------------------------

(Updated July 11, 2018, 11:52 a.m.)


Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.


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

Added helper for fetching review id.


Repository: mesos


Description (updated)
-------

Added helper for fetching review id.


Diffs (updated)
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/3/

Changes: https://reviews.apache.org/r/67503/diff/2-3/


Testing
-------


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added helper for fetching review id

Posted by Dragos Schebesch via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/
-----------------------------------------------------------

(Updated June 14, 2018, 11:13 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.


Repository: mesos


Description
-------

Added helper for fetching review id


Diffs (updated)
-----

  support/python3/get-review-ids.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67503/diff/2/

Changes: https://reviews.apache.org/r/67503/diff/1-2/


Testing
-------


Thanks,

Dragos Schebesch


Re: Review Request 67503: Added helper for fetching review id

Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67503/#review204669
-----------------------------------------------------------



I would rename the script `get-dependent-reviews.py` to be more explicit. Regarding the commit: its message should be more descriptive regarding the reasons behind this new support script and the Testing Done part should be filled with at least some tests or the fact that tests can be read later in the chain.


support/python3/get-review-ids.py
Lines 18 (patched)
<https://reviews.apache.org/r/67503/#comment287272>

    Missing docstring.



support/python3/get-review-ids.py
Lines 21 (patched)
<https://reviews.apache.org/r/67503/#comment287264>

    `# noqa`?
    Also, we generally make each import its own line (at least in the Python CLI codebase).



support/python3/get-review-ids.py
Lines 25 (patched)
<https://reviews.apache.org/r/67503/#comment287273>

    Missing docstring.



support/python3/get-review-ids.py
Lines 29 (patched)
<https://reviews.apache.org/r/67503/#comment287265>

    Why is it required? Not having an output file should result in having the dependent review IDs printed which is done in the last for loop of `support/python3/common.py`.



support/python3/get-review-ids.py
Lines 35 (patched)
<https://reviews.apache.org/r/67503/#comment287274>

    Missing docstring.


- Armand Grillet


On June 8, 2018, 12:07 p.m., Dragos Schebesch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67503/
> -----------------------------------------------------------
> 
> (Updated June 8, 2018, 12:07 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added helper for fetching review id
> 
> 
> Diffs
> -----
> 
>   support/python3/get-review-ids.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67503/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>