mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 62333: Added class for linters using a virtual environment.
Date Tue, 19 Sep 2017 22:35:58 GMT

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




support/mesos-style.py
Lines 209 (patched)
<https://reviews.apache.org/r/62333/#comment262085>

    How about VirtualenvLinterBase?
    
    But, do you need an abstract class for this? Why not just some functions?



support/mesos-style.py
Lines 214 (patched)
<https://reviews.apache.org/r/62333/#comment262088>

    Can you document what this returns? Or that rather than returning anything it handles
failure by exiting the program?



support/mesos-style.py
Lines 233 (patched)
<https://reviews.apache.org/r/62333/#comment262086>

    I'm left confused as a reader as to why this one takes a file list but build_virtualenv
does not take file list, what is the file list?
    
    Can you document this?



support/mesos-style.py
Lines 253-254 (patched)
<https://reviews.apache.org/r/62333/#comment262087>

    Is this clear to you? I don't understand why there is this distinction. My intuition would
tell me that we don't need to rebuild the virtualenv if it's already built, but of course
this code seems to suggest we always rebuild for a full lint?


- Benjamin Mahler


On Sept. 14, 2017, 3:34 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62333/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2017, 3:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
>     https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is used by the Python linter and
> will be used by the JavaScript linter.
> 
> 
> Diffs
> -----
> 
>   support/mesos-style.py cf37d9f4da4ab90b92f0136a1dcd5dd8bbae5785 
> 
> 
> Diff: https://reviews.apache.org/r/62333/diff/1/
> 
> 
> Testing
> -------
> 
> Manual updates of `.py` and `.js` files then test commits to see if the linters before
and afer removing their `.virtualenv` were still working as expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


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