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