Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-12-10 Thread Artem Harutyunyan

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

(Updated Dec. 10, 2015, 1:35 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Renamed variables to be more consistent.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py 6ea229facebf6ee071c0d0186fd1f702e0c48a39 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-12-10 Thread Artem Harutyunyan

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

(Updated Dec. 10, 2015, 1:42 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Fixed a typo.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py 6ea229facebf6ee071c0d0186fd1f702e0c48a39 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-14 Thread Artem Harutyunyan

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

(Updated Nov. 14, 2015, 9:02 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments, fixed commit options.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-14 Thread Artem Harutyunyan


> On Nov. 10, 2015, 12:18 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 192
> > 
> >
> > s/-e/--amend/ ?

Turns out `--amend` is very different from `-e`. `--amend` causes all commits 
in the branch to be squashed together into one. Excerpt from the man page 
(https://git-scm.com/docs/git-commit):

```
...
It is a rough equivalent for:


$ git reset --soft HEAD^
$ ... do something else to come up with the right tree ...
$ git commit -c ORIG_HEAD
```


- Artem


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


On Nov. 12, 2015, 10:03 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 12, 2015, 10:03 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-12 Thread Artem Harutyunyan

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

(Updated Nov. 12, 2015, 2:51 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-12 Thread Artem Harutyunyan

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

(Updated Nov. 12, 2015, 10:03 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-11 Thread Artem Harutyunyan

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

(Updated Nov. 11, 2015, 2:10 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-11 Thread Marco Massenzio


> On Nov. 10, 2015, 4:11 a.m., Kapil Arya wrote:
> > support/apply-reviews.py, lines 196-197
> > 
> >
> > Can we enhance it as following:
> > ```
> > amend=options['no_amend'] ? '' : '-e'
> > cmd = 'git commit --author \'{author}\' {amend} -am \'{message}\''\
> > .format(author=quote(data['author']),
> > amend=amend,
> > message=quote(data['message']))
> > ```
> > 
> > With this, we can now get rid of `amend_patch()`. This will speed up 
> > the process since we don't have to call `git commit --amend`. As we know, a 
> > call to `git commit --amend` without any staged file forces a full run of 
> > mesos-style.py, which can take several seconds.
> 
> Artem Harutyunyan wrote:
> TIL Python does not have a ternary operator, but point taken :).

Well, it has something that gets close enough :)
```
amend = '-e' if not options.get('no_amend') else ''
```
which I actually find more readable than the `? :` operator (and, apparently, 
Guido does too ;) )


- Marco


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


On Nov. 11, 2015, 10:10 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 11, 2015, 10:10 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Artem Harutyunyan

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

(Updated Nov. 9, 2015, 6:19 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Cleanup and rebase.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Vinod Kone

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

Ship it!



support/apply-reviews.py (line 42)


the comment only mentions Review Board but the code works for github too?



support/apply-reviews.py (lines 140 - 145)


More and more I look at this code, I feel like the global options is not a 
great idea. I think if these functions took explicit arguments instead of 
depending on global options, it would make it much easy to reason about.

Can you add a TODO for now to clean this up.



support/apply-reviews.py (line 143)


new line please.



support/apply-reviews.py (line 145)


new line please.



support/apply-reviews.py (line 149)


s/the//



support/apply-reviews.py (line 193)


Needs a comment on what this returns.



support/apply-reviews.py (lines 219 - 220)


indentation.



support/apply-reviews.py (lines 259 - 260)


indentation.



support/apply-reviews.py (line 275)


why the temp variable?


- Vinod Kone


On Nov. 9, 2015, 3:32 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 9, 2015, 3:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Kapil Arya

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



support/apply-reviews.py (lines 187 - 188)


Can we enhance it as following:
```
amend=options['no_amend'] ? '' : '-e'
cmd = 'git commit --author \'{author}\' {amend} -am \'{message}\''\
.format(author=quote(data['author']),
amend=amend,
message=quote(data['message']))
```

With this, we can now get rid of `amend_patch()`. This will speed up the 
process since we don't have to call `git commit --amend`. As we know, a call to 
`git commit --amend` without any staged file forces a full run of 
mesos-style.py, which can take several seconds.


- Kapil Arya


On Nov. 9, 2015, 10:32 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 9, 2015, 10:32 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Artem Harutyunyan

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

(Updated Nov. 9, 2015, 7:32 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Artem Harutyunyan

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

(Updated Nov. 9, 2015, 11:48 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py d39ee9bb0ee782bd756b7a5fc0dec70d056c9589 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Artem Harutyunyan


> On Nov. 9, 2015, 8:11 p.m., Kapil Arya wrote:
> > support/apply-reviews.py, lines 196-197
> > 
> >
> > Can we enhance it as following:
> > ```
> > amend=options['no_amend'] ? '' : '-e'
> > cmd = 'git commit --author \'{author}\' {amend} -am \'{message}\''\
> > .format(author=quote(data['author']),
> > amend=amend,
> > message=quote(data['message']))
> > ```
> > 
> > With this, we can now get rid of `amend_patch()`. This will speed up 
> > the process since we don't have to call `git commit --amend`. As we know, a 
> > call to `git commit --amend` without any staged file forces a full run of 
> > mesos-style.py, which can take several seconds.

TIL Python does not have a ternary operator, but point taken :).


- Artem


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


On Nov. 9, 2015, 7:32 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Nov. 9, 2015, 7:32 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3859
> https://issues.apache.org/jira/browse/MESOS-3859
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Artem Harutyunyan

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

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


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-09 Thread Artem Harutyunyan

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

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


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-04 Thread Artem Harutyunyan


> On Nov. 2, 2015, 4:55 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 132
> > 
> >
> > this is weird too. a 'remove patch' function which takes optional patch 
> > as an argument.

Added a comment to clarify.


> On Nov. 2, 2015, 4:55 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, lines 137-140
> > 
> >
> > This log shouldn't be in the caller instead of this function.

It would be trivial to move the check and the comment to a calling function, 
however it would mean that we will be making an exception and passing the 
`dry_run` argument down from the higher level function. I refactored the code, 
added an `if` statement. Hopefully it makes it easier to read and makes the 
intent more explicit. Please let me know if you still believe that this should 
be improved further.


> On Nov. 2, 2015, 4:55 p.m., Vinod Kone wrote:
> > support/apply-reviews.py, line 48
> > 
> >
> > i have seen this in the previous reviews too. so wanted to confirm.
> > 
> > why do you use urlparse.urljoin() for the latter part of this url, but 
> > use "/" for the former part?

>From what I've seen `urljoin` does not support just joing parts of URLs. It 
>treats the first part as a base, and so it's really not obvious how one can 
>use it to do what I need to do. I am going to go ahead and get rid of it 
>altogether. I think I'll just use .format everywhere.


- Artem


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


On Oct. 30, 2015, 1:55 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Oct. 30, 2015, 1:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-04 Thread Artem Harutyunyan

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

(Updated Nov. 4, 2015, 3 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-11-02 Thread Vinod Kone

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



support/apply-reviews.py (line 47)


i have seen this in the previous reviews too. so wanted to confirm.

why do you use urlparse.urljoin() for the latter part of this url, but use 
"/" for the former part?



support/apply-reviews.py (lines 104 - 108)


this is really confusing. why do you need both a 'dry_run' and a 
'ignore_dry_run' argument?

honestly i liked the previous version.



support/apply-reviews.py (line 125)


this is weird too. a 'remove patch' function which takes optional patch as 
an argument.



support/apply-reviews.py (lines 130 - 133)


This log shouldn't be in the caller instead of this function.



support/apply-reviews.py (line 152)


does this only fetch from review board or github too?



support/apply-reviews.py (lines 162 - 164)


instead of a second parameter, have shell() function take one argument 
'dry_run' and act according to it, instead of looking into the global 'options'.

also, this says "in case of github" but it is doing the same for 
reviewboard too? either the 1) comment needs to be fixed or 2) this should be 
in a 'if else'.


- Vinod Kone


On Oct. 30, 2015, 8:55 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Oct. 30, 2015, 8:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-30 Thread Artem Harutyunyan

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

(Updated Oct. 30, 2015, 1:55 a.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-28 Thread Artem Harutyunyan

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

(Updated Oct. 28, 2015, 3:08 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed (some) comments.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-27 Thread Vinod Kone

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



support/apply-reviews.py (line 38)


either this function should renamed to something review board specific or 
the function need to get both RB and GH users.



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


update comment?



support/apply-reviews.py (lines 76 - 78)


bad rebase.



support/apply-reviews.py (line 105)


if 'dry_run' in options:



support/apply-reviews.py (lines 149 - 155)


i don't follow. why not just call 'shell(cmd)' if you want to override 
'dry_run'? is it because 'shell()' doesn't print the command?



support/apply-reviews.py (line 177)


s/populate_//



support/apply-reviews.py (line 192)


s/populate_//



support/apply-reviews.py (line 218)


s/populate_//



support/apply-reviews.py (line 228)


indentation?



support/apply-reviews.py (line 268)


s/mututally/mutually/ ?


- Vinod Kone


On Oct. 23, 2015, 11:13 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Oct. 23, 2015, 11:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Marco Massenzio


> On Oct. 20, 2015, 5:06 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 160
> > 
> >
> > you don't really need to escape the quotes, just take advantage of 
> > Python being able to use single and double quotes interchangeably (or even 
> > use """ if you really want to be hip :)
> 
> Artem Harutyunyan wrote:
> I actually do need to escape the quotes becasue {message} is multiline 
> and I am executing the `cmd` in a shell.
> 
> Marco Massenzio wrote:
> I don't understand your comment, I'm afraid. What I did mean was that you 
> can write:
> ```
> cmd = "git commit --author '{author}' -am '{message}'".format(author = 
> review['author'], ...)
> ```
> and this will have the exact same meaning as your code (whether that's 
> correctly escaped and executed in the shell, I really can't say).
> 
> Artem Harutyunyan wrote:
> I see what you're saying. I am using single quotes throughout the code, 
> that's why I preferred quoting to switching to double quotes for these 
> particular instances.

ah - that sounds reasonable (although, you'll be relieved to know that PEP8 is 
silent on that point :)
The general approach is to use both, being consistent across "usage classes."


- Marco


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


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



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Marco Massenzio

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

Ship it!


This looks great - a really few minor nits, but then it's good to go!


support/apply-reviews.py (lines 47 - 48)


s/cerror/error

However, I think it'd read better as
```
# Trailing slash required by ReviewBoard's REST API
```



support/apply-reviews.py (line 49)


```
if 'review_id' in options:
```
is more pythonic
(also below: `elif 'github' in options`)



support/apply-reviews.py (line 262)


you don't need the trailing \
also, please indent the second row, keeping the opening quotes aligned:
```
description='foo bar baz '
'jump the fox')
```



support/apply-reviews.py (lines 287 - 290)


does this work? (I really am curious, I didn't know about the 'mutually 
exclusive' option!)

in other words, don't you get an exception by trying to access **both** 
`args.github` AND `args.review_id` (because one of the two is missing)?


- Marco Massenzio


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



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Artem Harutyunyan


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 160
> > 
> >
> > you don't really need to escape the quotes, just take advantage of 
> > Python being able to use single and double quotes interchangeably (or even 
> > use """ if you really want to be hip :)
> 
> Artem Harutyunyan wrote:
> I actually do need to escape the quotes becasue {message} is multiline 
> and I am executing the `cmd` in a shell.
> 
> Marco Massenzio wrote:
> I don't understand your comment, I'm afraid. What I did mean was that you 
> can write:
> ```
> cmd = "git commit --author '{author}' -am '{message}'".format(author = 
> review['author'], ...)
> ```
> and this will have the exact same meaning as your code (whether that's 
> correctly escaped and executed in the shell, I really can't say).

I see what you're saying. I am using single quotes throughout the code, that's 
why I preferred quoting to switching to double quotes for these particular 
instances.


- Artem


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


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



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Artem Harutyunyan


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, lines 277-282
> > 
> >
> > this code look familiar and I remember already commenting about 
> > `applied` :)
> 
> Artem Harutyunyan wrote:
> Yep, it used to be a `dict`, and I changed it to a `set`. Did I miss 
> anything?
> 
> Marco Massenzio wrote:
> sorry - I wasn't paying attention.
> all good.

Nope, I just wanted to make sure that I did not miss anything :).


- Artem


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


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



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Marco Massenzio


> On Oct. 20, 2015, 5:06 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 160
> > 
> >
> > you don't really need to escape the quotes, just take advantage of 
> > Python being able to use single and double quotes interchangeably (or even 
> > use """ if you really want to be hip :)
> 
> Artem Harutyunyan wrote:
> I actually do need to escape the quotes becasue {message} is multiline 
> and I am executing the `cmd` in a shell.

I don't understand your comment, I'm afraid. What I did mean was that you can 
write:
```
cmd = "git commit --author '{author}' -am '{message}'".format(author = 
review['author'], ...)
```
and this will have the exact same meaning as your code (whether that's 
correctly escaped and executed in the shell, I really can't say).


> On Oct. 20, 2015, 5:06 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, lines 277-282
> > 
> >
> > this code look familiar and I remember already commenting about 
> > `applied` :)
> 
> Artem Harutyunyan wrote:
> Yep, it used to be a `dict`, and I changed it to a `set`. Did I miss 
> anything?

sorry - I wasn't paying attention.
all good.


- Marco


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


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



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Marco Massenzio


> On Oct. 20, 2015, 5:06 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 106
> > 
> >
> > it would be really nice if we could make our code work under both 2.7 
> > and 3.x Python ;)
> 
> Artem Harutyunyan wrote:
> replaced this with `print(output)`, that should work in 3.x, correct?

yes, but only by mistake :)
(`(output)` is interpreted as a 1-tuple in 2.7 and `print` is still assumed to 
be without parentheses - in 3.4, as a paren-surrounded string)

You will need to add (as the VERY FIRST line after the shebang):
```
from __future__ import print_function
```


- Marco


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


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



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Artem Harutyunyan


> On Oct. 23, 2015, 12:18 a.m., Marco Massenzio wrote:
> > support/apply-reviews.py, lines 287-290
> > 
> >
> > does this work? (I really am curious, I didn't know about the 'mutually 
> > exclusive' option!)
> > 
> > in other words, don't you get an exception by trying to access **both** 
> > `args.github` AND `args.review_id` (because one of the two is missing)?

It does :-). `args` sets the value of the missing option to `None`.


- Artem


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


On Oct. 23, 2015, 4:13 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Oct. 23, 2015, 4:13 p.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-23 Thread Artem Harutyunyan

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

(Updated Oct. 23, 2015, 4:13 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-21 Thread Artem Harutyunyan

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

(Updated Oct. 20, 2015, 11:04 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-21 Thread Artem Harutyunyan


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 47
> > 
> >
> > could you please add an @param and explain what `options` is and what 
> > does it look like?
> > (also an @type would be awesome)

There is a comment about `options` in `parse_options`, and the fields names are 
self-explanatory. I feel like if I add it here, I will need to go and add it 
everywhere else, and I'd like to avoid repeating `@param` for all funtions. I 
am happy to consider other options though.


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 106
> > 
> >
> > it would be really nice if we could make our code work under both 2.7 
> > and 3.x Python ;)

replaced this with `print(output)`, that should work in 3.x, correct?


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 160
> > 
> >
> > you don't really need to escape the quotes, just take advantage of 
> > Python being able to use single and double quotes interchangeably (or even 
> > use """ if you really want to be hip :)

I actually do need to escape the quotes becasue {message} is multiline and I am 
executing the `cmd` in a shell.


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, lines 277-282
> > 
> >
> > this code look familiar and I remember already commenting about 
> > `applied` :)

Yep, it used to be a `dict`, and I changed it to a `set`. Did I miss anything?


- Artem


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


On Oct. 20, 2015, 12:45 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39410/
> ---
> 
> (Updated Oct. 20, 2015, 12:45 a.m.)
> 
> 
> Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for github to apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39410/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-20 Thread Artem Harutyunyan

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

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


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-19 Thread Marco Massenzio

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



support/apply-reviews.py (line 23)


hey, as mentioned, could you please remove whitespace around `=` in args 
call (and default params)

here and everywhere.

thanks!



support/apply-reviews.py (line 27)


is this right? by looking at the args names, looks like it returns the URL 
of a PR given its number?

either the comment or the args names are wrong.

also, I would much prefer `pull_request_number`



support/apply-reviews.py (line 44)


could you please add an @param and explain what `options` is and what does 
it look like?
(also an @type would be awesome)



support/apply-reviews.py (lines 46 - 47)


is this dead code? please remove.



support/apply-reviews.py (line 48)


this will cause a KeyError if `reveiw_id` is not there.
you can achieve the same result (but better) with:
```
if options.get('review_id'):
```
(this also protects you further below in the call to `format`)



support/apply-reviews.py (line 97)


```
if not options or 'dry_run' not in options:
```



support/apply-reviews.py (line 103)


it would be really nice if we could make our code work under both 2.7 and 
3.x Python ;)



support/apply-reviews.py (line 133)


this line exceeds 100 char (I'm almost sure)
you can break it using \

or, even better, build the command via string.join():
```
' '.join(['wget',
  '--no-check-certificate',
  '--no-verbose',
  '-O {}.patch'.format(patch_id(options)),
  patch_url(options)])
```



support/apply-reviews.py (line 136)


s/beacuse/because



support/apply-reviews.py (line 145)


unnecessary parentheses; also, please use `get()` instead of `[]`
(unless you are terminally positive both keys are there every time)



support/apply-reviews.py (line 156)


you don't really need to escape the quotes, just take advantage of Python 
being able to use single and double quotes interchangeably (or even use """ if 
you really want to be hip :)



support/apply-reviews.py (lines 179 - 181)


this looks a bit ugly - prefer:
```
message = '{summary}\n\n{description}\n\nThis closes: {pr}'.
format(summary=title, description=description, pr=pr_number)
```



support/apply-reviews.py (lines 184 - 188)


no space before `:`

(also, it's more "pythonic" to use double quotes for the dict's keys - but 
really a micro-nit!)



support/apply-reviews.py (lines 200 - 205)


could you please reformat this code to something more readable?
```
username = review.get('links').get('submitter').get('title')
user = url_to_json(user_url(username)).get('user')
url = review_url(rid)
author = '{author} {email}'.format(author=user.get('fullname'), 
   email=user.get('email'))
message = '\n\n'.join([review.get('summary'),
   review.get('description'),
   'Review: {}'.format(url)
```



support/apply-reviews.py (lines 251 - 255)


Much better:
```  
options = {
'review_id': args.review_id,
'dry_run': args.dry_run,
'no_amend': args.no_amend,
'github': args.github
}
```
or if you really want to be pythonic:
```
options = dict()
for k in ['review_id', 'dry_run', 'no_amend', 'github']:
options[k] = args.getattr(k)
```
although, I'm not sure right now if this will work with your having made 
`github` and `review_id` mutually exclusive (but, then again, the dotted 
notation should blow up too if the arg is not there?)



support/apply-reviews.py (lines 265 - 270)


this code look familiar and I remember already commenting about `applied` :)


- Marco Massenzio


On Oct. 18, 2015, 10:32 p.m., Artem Harutyunyan wrote:
> 
> 

Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-18 Thread Artem Harutyunyan

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

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


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

space/tab cleanup.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan



Re: Review Request 39410: Added support for github to apply-reviews.py.

2015-10-16 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [38705, 38883, 39410]

All tests passed.

- Mesos ReviewBot


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