mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Artem Harutyunyan" <ar...@mesosphere.io>
Subject Re: Review Request 39410: Added support for github to apply-reviews.py.
Date Wed, 21 Oct 2015 06:03:50 GMT


> On Oct. 19, 2015, 10:06 p.m., Marco Massenzio wrote:
> > support/apply-reviews.py, line 47
> > <https://reviews.apache.org/r/39410/diff/3/?file=1100642#file1100642line47>
> >
> >     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
> > <https://reviews.apache.org/r/39410/diff/3/?file=1100642#file1100642line106>
> >
> >     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
> > <https://reviews.apache.org/r/39410/diff/3/?file=1100642#file1100642line160>
> >
> >     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
> > <https://reviews.apache.org/r/39410/diff/3/?file=1100642#file1100642line277>
> >
> >     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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message