Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Nov. 9, 2015, 2:52 a.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

More cleanup.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Nov. 9, 2015, 12:28 a.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Cleanups.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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

Ship it!



support/apply-reviews.py (line 45)


You could add a newline at the end of this string.  (`write` doesn't do 
that for you.)



support/apply-reviews.py (line 57)


Newline at end of string.


- Joseph Wu


On Oct. 28, 2015, 2:57 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 28, 2015, 2:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Oct. 28, 2015, 2:57 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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


Note: considering the amount of changes since my last review, my "Ship It" is 
no longer valid.


support/apply-reviews.py (line 15)


This won't work on Windows, since it'll construct "URL"s with (evil) 
backslashes.
(And considering how often we use this script for the CMake/Windows review 
chains... :)

This would be better:
https://docs.python.org/2/library/urlparse.html#urlparse.urljoin



support/apply-reviews.py (line 38)


Nit: Extra space after the if.



support/apply-reviews.py (line 78)


Nit: Extra space after function name.


- Joseph Wu


On Oct. 22, 2015, 11:16 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 22, 2015, 11:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Vinod Kone]

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

Ship it!


modulo joseph's comments.


support/apply-reviews.py (line 57)


log the id of the violating review?


- Vinod Kone


On Oct. 23, 2015, 6:16 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 23, 2015, 6:16 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Vinod Kone]

> On Oct. 22, 2015, 9:20 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 25
> > 
> >
> > s/extract_//
> 
> Artem Harutyunyan wrote:
> Marco commented earlier on this one `nit: you are 'masking' the global 
> builtin id() here - that's a PEP8 violation, could you please rename to 
> something like rid?`.

is the concern that if you call this method review_id(), you cannot use 
"review_id" as a variable in the method? if yes, you can call the local 
variable 'rid'.


- Vinod


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


On Oct. 23, 2015, 6:16 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 23, 2015, 6:16 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

> On Oct. 22, 2015, 2:20 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 25
> > 
> >
> > s/extract_//

Marco commented earlier on this one `nit: you are 'masking' the global builtin 
id() here - that's a PEP8 violation, could you please rename to something like 
rid?`.


> On Oct. 22, 2015, 2:20 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 32
> > 
> >
> > s/parent_review/review_chain/ ?

That's exactly what I renamed that function to two reviews down the chain.


- Artem


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


On Oct. 22, 2015, 11:16 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 22, 2015, 11:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Vinod Kone]

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



support/apply-reviews.py (line 25)


s/extract_//



support/apply-reviews.py (line 32)


s/parent_review/review_chain/ ?



support/apply-reviews.py (lines 41 - 42)


Shouldn't this comment be rephrased/killed since you are disallowing more 
than one immediate parent below?



support/apply-reviews.py (line 56)


why extract summary into a variable here? you didn't do that above in #51.



support/apply-reviews.py (line 57)


what if there is a circular dependency of reviews by mistake?

A --> B --> C --> A



support/apply-reviews.py (line 58)


Why do you need to take 'r_list' as an argument to this method? to catch 
circular dependencies? if not, you could've done something as follows?

```
def review_list(review_id):

  review = review_json(review_id)
  
  if len (review.depends) == 0 :
 return [review_id , review.summary]
  elif len (review.depends) == 1:
 return review_list(review.depends.id) + [review_id, review.summary]
  else:
 error()
```


- Vinod Kone


On Oct. 20, 2015, 7:28 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 20, 2015, 7:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Oct. 20, 2015, 12:28 a.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Vinod Kone]

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



support/apply-reviews.py (line 12)


s/rid/review_id/

here and everywhere else.



support/apply-reviews.py (line 22)


why not have this return JSON instead of string?



support/apply-reviews.py (line 25)


s/extract/review/



support/apply-reviews.py (line 32)


s/r_list/reviews/

also, the function name is misleading. how about

s/parent_review/review_chain/ ?



support/apply-reviews.py (lines 33 - 34)


what does "reversed chain of review requests for a  given Review ID" mean? 
i'm assuming you mean dependent reviews? please make that more clear.



support/apply-reviews.py (lines 35 - 36)


if review_json returns json, this could be

json = review_json(review_url(rid))



support/apply-reviews.py (line 43)


s/A may/A review may/

do we want to allow reviews that have more than one parent review? what 
does that mean for the order of reviews to be applied? i think it makes sense 
to disallow such reviews. throw an error.



support/apply-reviews.py (lines 49 - 52)


i don't follow what's happening here. you are appending the same 
 pair multiple times to the list?



support/apply-reviews.py (line 56)


2 lines.



support/apply-reviews.py (line 75)


seems weird to have this function return the command if dry_run is true and 
None otherwise.

how about just prefixing "echo" to the command if dry_run is true?



support/apply-reviews.py (line 101)


s/print //


- Vinod Kone


On Oct. 18, 2015, 10:30 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 18, 2015, 10:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Oct. 18, 2015, 3:30 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

space/tab cleanup.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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


Thanks for all the tips, Marco\! I learned a lot of Python here :).

- Artem Harutyunyan


On Oct. 15, 2015, 11:50 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 15, 2015, 11:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

> On Oct. 14, 2015, 5:55 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 7
> > 
> >
> > uhm... could we use `requests` instead?
> > much more modern API and widespread use.

`requests` looks great, but it looks like it requires to be installed 
separately. I would really like to restrict myself to only the modules 
available with stock python distribution. Ditto for `sh`.


> On Oct. 14, 2015, 5:55 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 19
> > 
> >
> > you should check for a non 4xx/5xx return code (or just check for a 2xx 
> > if you know for sure what the API returns).
> > 
> > as you expect JSON, you should probably specify that in the 
> > `Accept-content` header too?

urllib2 throws an exception if an error occurs, which forces the program to 
terminate. Termination is the right thing to do here, because there is no 
recovery path, so I would only want to catch the exception to print a pretty 
message, but the stack trace should be informative enough for the developer who 
is running this script. 
As for the `content-type`, ReviewBoard is setting it to something unorthodox 
(`Content-Type: application/vnd.reviewboard.org.review-request+json`), do you 
think we should verify against that specific value?


> On Oct. 14, 2015, 5:55 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 24
> > 
> >
> > if you are doing this often enough, it's much better to compile this to 
> > a constant Pattern and then use that instead.

It's done only once per review, so compiling will not give us any significant 
performance increase.


> On Oct. 14, 2015, 5:55 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 45
> > 
> >
> > is this right?
> > you return after the first iteration?
> > 
> > if so, why not just getting the first item in `depends_on`?

I think this is right. That statement is after the recursive call, so actually 
when in gets there for the last time the lisst contains the entire chain 
(whereas `depends_on` only contains immediate anchestor(s)).


> On Oct. 14, 2015, 5:55 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 56
> > 
> >
> > if you do this, this script will be probably useful to folks who use 
> > Python 3 too :)
> > 
> > ```
> > from __future__ import print_function
> > 
> > ...
> > 
> > print('foo bar')
> > ```
> > Also (but that's just something a bunch of us insisted upon) I prefer 
> > the use of `string.format()` to `%`:
> > ```
> > print("Applying review {}: {}".format(review_id, summary))
> > ```
> > (same also below to build `cmd`)

I am not sure whether we should use python 3. Other python scripts in Mesos 
repo seem to be written for 2.x versions, so I'd like to stay consistent.


- Artem


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


On Oct. 8, 2015, 11:26 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 8, 2015, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Oct. 15, 2015, 11:50 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Marco Massenzio]

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



support/apply-reviews.py (line 7)


uhm... could we use `requests` instead?
much more modern API and widespread use.



support/apply-reviews.py (line 11)


nit: you are 'masking' the global builtin `id()` here - that's a PEP8 
violation, could you please rename to something like `rid`?



support/apply-reviews.py (line 13)


don't concatenate string, prefer the use of os.path.join():
```
import os

os.path.join(REVIEW_URL, rid)
```
(why do you need the trailing slash?)



support/apply-reviews.py (line 19)


you should check for a non 4xx/5xx return code (or just check for a 2xx if 
you know for sure what the API returns).

as you expect JSON, you should probably specify that in the 
`Accept-content` header too?



support/apply-reviews.py (line 24)


if you are doing this often enough, it's much better to compile this to a 
constant Pattern and then use that instead.



support/apply-reviews.py (line 29)


you should not call a param with the name of a type (`list` is a type in 
Python).
also, it's bad practice to initialize it as you do there.

BTW - can you please use the @param to explain what the method expects?

Prefer:
```
def parent_review(rid, revs_list=None):
  """Returns a list with reversed chain of review requests for a given
  Review ID.
  """
  result = revs_list or []
  json_str = review_json(review_url(rid))
  json_obj = json.loads(json_str)

  # Stop as soon as we stumble upon a submitted request.
  rr = json_obj.get('review_request')
  if rr and rr.get('status') == "submitted":
return result

  for deps in rr.get('summary'):
  ...
  
```

Also, worth always using `get()` instead of accessor (`[]`) for data that 
you are not sure may or may not be there (it's an API response after all) - the 
latter will throw a KeyError if the key is missing; `get` will just give you 
back None (or you can pass a second optional default return value if you want).



support/apply-reviews.py (line 40)


is this a recursion inside an iteration?
can you please add a comment? I'm sure folks who may one day try to 
fix/augment this script will be just as confused...



support/apply-reviews.py (line 45)


is this right?
you return after the first iteration?

if so, why not just getting the first item in `depends_on`?



support/apply-reviews.py (line 56)


if you do this, this script will be probably useful to folks who use Python 
3 too :)

```
from __future__ import print_function

...

print('foo bar')
```
Also (but that's just something a bunch of us insisted upon) I prefer the 
use of `string.format()` to `%`:
```
print("Applying review {}: {}".format(review_id, summary))
```
(same also below to build `cmd`)



support/apply-reviews.py (line 90)


nit: `if r not in applied:` is more pythonic :)

(also, PEP8 doesn't really like 1- and 2-letter variables)



support/apply-reviews.py (line 91)


do you need a counter or something like that, or a flag would suffice?
I'm not sure how you use `applied` as a dict (looks like you are just using 
it as a `set`?):
```
applied = set()
for review, summary in reviews:
if review not in applied:
applied.add(review)
print(apply_review(...))
```


- Marco Massenzio


On Oct. 8, 2015, 6:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 8, 2015, 6:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> 

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Guangya Liu]

> On 十月 1, 2015, 1:12 p.m., Guangya Liu wrote:
> > support/apply-reviews.py, line 2
> > 
> >
> > The import should be in alpha order
> 
> Artem Harutyunyan wrote:
> I am happy to fix this, but could you please justify your reasoning?

Please check 
https://github.com/apache/mesos/blob/master/support/post-reviews.py#L25-#L30 

Also I see that most of the import or include are using alpha order in mesos.


- Guangya


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


On 十月 1, 2015, 5:12 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated 十月 1, 2015, 5:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[haosdent huang]

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


IMHO, we keep both apply-reviews.py and apply-reviews.sh and call 
apply-reviews.sh in apply-reviews.py looks strange. It would be better we add 
this function to apply-review.sh. Or create apply-reviews.py and deprecated 
apply-reviews.sh

- haosdent huang


On Oct. 1, 2015, 5:12 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Oct. 1, 2015, 5:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Guangya Liu]

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



support/apply-reviews.py (line 2)


The import should be in alpha order


- Guangya Liu


On 九月 30, 2015, 6:09 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated 九月 30, 2015, 6:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Oct. 1, 2015, 10:12 a.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

> On Oct. 1, 2015, 6:12 a.m., Guangya Liu wrote:
> > support/apply-reviews.py, line 2
> > 
> >
> > The import should be in alpha order

I am happy to fix this, but could you please justify your reasoning?


- Artem


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


On Sept. 29, 2015, 11:09 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 29, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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

Ship it!


LGTM!

Note: The long-form cmd-line arguments (i.e. `--dry-run`) are not really 
necessary, but might be nice.


support/apply-reviews.py (line 12)


Typo: Retutns


- Joseph Wu


On Sept. 29, 2015, 11:09 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 29, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> Tested the script with python 2.7.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Sept. 29, 2015, 11:09 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing (updated)
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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

Ship it!



support/apply-reviews.py (line 8)


Not used.



support/apply-reviews.py (line 12)


Typo: Retutns.

Also, docstrings go inside the method body.

```
def review_url(id):
  """Returns a Review Board URL given a review ID."""
```

You can tell if it is correct if you do something like:
```
python
import apply-reviews
print review_url.__doc__
```



support/apply-reviews.py (line 18)


s/JSON-ifyed/JSON-ified/


- Joseph Wu


On Sept. 26, 2015, 7:02 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 26, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38705]

All tests passed.

- Mesos ReviewBot


On Sept. 27, 2015, 2:02 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 27, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

> On Sept. 24, 2015, 5:38 p.m., Joseph Wu wrote:
> > support/apply-reviews.py, line 13
> > 
> >
> > Might be cleaner/safer to use urlparse.urljoin for this:
> > https://docs.python.org/2/library/urlparse.html#urlparse.urljoin

`urlparse.urljoin` does not seem to do exactly what is needed here.


> On Sept. 24, 2015, 5:38 p.m., Joseph Wu wrote:
> > support/apply-reviews.py, line 68
> > 
> >
> > Seems like you don't need a `-r` in this case, because we'd only use 
> > this script for reviewboard.
> > 
> > i.e. s/-r/review/

I have it for compatibility with `apply-review.sh` and also in case we decide 
to add support for github later.


- Artem


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


On Sept. 23, 2015, 9:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 23, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Sept. 26, 2015, 7:02 p.m.)


Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Addressed comments; changed tab size to 2 spaces.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Vinod Kone]

> On Sept. 25, 2015, 4:35 p.m., Joseph Wu wrote:
> > One more thing: when the patch fails to apply, I get this message:
> > 
> > ```
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 86, in 
> > apply_review(r, dry_run)
> >   File "support/apply-reviews.py", line 60, in apply_review
> > shell(cmd)
> >   File "support/apply-reviews.py", line 53, in shell
> > command, stderr=subprocess.STDOUT, shell=True)
> >   File 
> > "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
> >  line 573, in check_output
> > raise CalledProcessError(retcode, cmd, output=output)
> > subprocess.CalledProcessError: Command './support/apply-review.sh -n -r 
> > 38752' returned non-zero exit status 1
> > ```
> > 
> > Instead of this one:
> > ```
> > 2015-09-25 09:34:05 URL:https://reviews.apache.org/r/38752/diff/raw/ 
> > [1426/1426] -> "38752.patch" [1]
> > error: patch failed: 3rdparty/libprocess/3rdparty/CMakeLists.txt:87
> > error: 3rdparty/libprocess/3rdparty/CMakeLists.txt: patch does not apply
> > error: patch failed: cmake/MesosConfigure.cmake:114
> > error: cmake/MesosConfigure.cmake: patch does not apply
> > Failed to apply patch
> > ```

what is support/apply-reviews.py? don't think its committed yet.


- Vinod


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


On Sept. 24, 2015, 4:26 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 24, 2015, 4:26 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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


One more thing: when the patch fails to apply, I get this message:

```
Traceback (most recent call last):
  File "support/apply-reviews.py", line 86, in 
apply_review(r, dry_run)
  File "support/apply-reviews.py", line 60, in apply_review
shell(cmd)
  File "support/apply-reviews.py", line 53, in shell
command, stderr=subprocess.STDOUT, shell=True)
  File 
"/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
 line 573, in check_output
raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command './support/apply-review.sh -n -r 38752' 
returned non-zero exit status 1
```

Instead of this one:
```
2015-09-25 09:34:05 URL:https://reviews.apache.org/r/38752/diff/raw/ 
[1426/1426] -> "38752.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/CMakeLists.txt:87
error: 3rdparty/libprocess/3rdparty/CMakeLists.txt: patch does not apply
error: patch failed: cmake/MesosConfigure.cmake:114
error: cmake/MesosConfigure.cmake: patch does not apply
Failed to apply patch
```

- Joseph Wu


On Sept. 23, 2015, 9:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 23, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

> On Sept. 25, 2015, 9:35 a.m., Joseph Wu wrote:
> > One more thing: when the patch fails to apply, I get this message:
> > 
> > ```
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 86, in 
> > apply_review(r, dry_run)
> >   File "support/apply-reviews.py", line 60, in apply_review
> > shell(cmd)
> >   File "support/apply-reviews.py", line 53, in shell
> > command, stderr=subprocess.STDOUT, shell=True)
> >   File 
> > "/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py",
> >  line 573, in check_output
> > raise CalledProcessError(retcode, cmd, output=output)
> > subprocess.CalledProcessError: Command './support/apply-review.sh -n -r 
> > 38752' returned non-zero exit status 1
> > ```
> > 
> > Instead of this one:
> > ```
> > 2015-09-25 09:34:05 URL:https://reviews.apache.org/r/38752/diff/raw/ 
> > [1426/1426] -> "38752.patch" [1]
> > error: patch failed: 3rdparty/libprocess/3rdparty/CMakeLists.txt:87
> > error: 3rdparty/libprocess/3rdparty/CMakeLists.txt: patch does not apply
> > error: patch failed: cmake/MesosConfigure.cmake:114
> > error: cmake/MesosConfigure.cmake: patch does not apply
> > Failed to apply patch
> > ```
> 
> Vinod Kone wrote:
> what is support/apply-reviews.py? don't think its committed yet.

I used the `apply-reviews.py` from this review (applied normally via 
`apply-review.sh`) to apply a separate chain (which was ~15 reviews long).

(Meta-apply :)


- Joseph


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


On Sept. 23, 2015, 9:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 23, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38705]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 4:26 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 24, 2015, 4:26 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Joseph Wu]

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


Mostly just python style and commenting.  My python is a bit rusty though.

Could you also mention in "Testing Done" whether this is python 2 or 3 or both?


support/apply-reviews.py (line 9)


Spaces around `=`?



support/apply-reviews.py (line 11)


Could you change these method comments to python docstrings comments?

i.e.
```
def review_url(id):
"""Returns the Review Board URL for the given review ID."""
```



support/apply-reviews.py (line 13)


Might be cleaner/safer to use urlparse.urljoin for this:
https://docs.python.org/2/library/urlparse.html#urlparse.urljoin



support/apply-reviews.py (line 15)


Docstring-ify.

Looks like this method is just json-ifying an HTTP GET from the url.  
(Reword comment?)



support/apply-reviews.py (line 20)


Docstring-ify.



support/apply-reviews.py (lines 26 - 28)


Docstring-ify.

Is the list of reviews in reverse order of the chain?



support/apply-reviews.py (line 29)


I believe the python convention for (not-really) "private" helper methods 
is a leading underscore.

i.e. `def _parent_review(...)`



support/apply-reviews.py (line 40)


Extra newline here.



support/apply-reviews.py (lines 44 - 45)


Docstring-ify.



support/apply-reviews.py (line 46)


Seems like you could combine this into `parent_review_ex` by changing the 
function prototype to:

`parent_review(id, list=[])`



support/apply-reviews.py (line 49)


Docstring-ify.



support/apply-reviews.py (line 55)


Docstring-ify.



support/apply-reviews.py (line 56)


It would be great if this included a snippet of the review's summary.



support/apply-reviews.py (line 68)


Seems like you don't need a `-r` in this case, because we'd only use this 
script for reviewboard.

i.e. s/-r/review/



support/apply-reviews.py (line 72)


Add `'--dry-run'` to this option?


- Joseph Wu


On Sept. 23, 2015, 9:26 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38705/
> ---
> 
> (Updated Sept. 23, 2015, 9:26 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

Review request for mesos, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---


Thanks,

Artem Harutyunyan