mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 39410: Added support for github to apply-reviews.py.
Date Tue, 03 Nov 2015 00:55:28 GMT

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



support/apply-reviews.py (line 47)
<https://reviews.apache.org/r/39410/#comment163087>

    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)
<https://reviews.apache.org/r/39410/#comment163088>

    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)
<https://reviews.apache.org/r/39410/#comment163094>

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



support/apply-reviews.py (lines 130 - 133)
<https://reviews.apache.org/r/39410/#comment163096>

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



support/apply-reviews.py (line 152)
<https://reviews.apache.org/r/39410/#comment163098>

    does this only fetch from review board or github too?



support/apply-reviews.py (lines 162 - 164)
<https://reviews.apache.org/r/39410/#comment163097>

    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
> 
>


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