----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62214/#review185102 -----------------------------------------------------------
Can you add Kevin Klues to the review and ask him to take a look at the virtual env bootstrapping? I'm assuming you based this on his one for the CLI? src/webui/master/static/.eslintrc.js Lines 6 (patched) <https://reviews.apache.org/r/62214/#comment261338> Does this run successfully on the webui? Can you document for posterity how this config was produced? src/webui/master/static/.gitignore Lines 1 (patched) <https://reviews.apache.org/r/62214/#comment261336> Ditto here re: serving the file over HTTP. src/webui/master/static/bootstrap Lines 3-4 (patched) <https://reviews.apache.org/r/62214/#comment261335> I think the master serves all assets under static: https://github.com/apache/mesos/blob/297b7042a7ef65aafca40832b5f8736e27e26ed2/src/master/master.cpp#L1122 Which means that if this file is here it's served by the master. support/mesos-style.py Lines 298-302 (patched) <https://reviews.apache.org/r/62214/#comment261331> Stale copy paste support/mesos-style.py Lines 307-311 (patched) <https://reviews.apache.org/r/62214/#comment261334> Similarly to my comment below, if we had some virtual env abstraction this could be running something within in: ``` virtualenv.run_within(['eslint', ...]); ``` support/mesos-style.py Lines 309 (patched) <https://reviews.apache.org/r/62214/#comment261339> Is this applying a lint on the whole file or just the diff? Is it possible to do just the diff with eslint? If not, we'll need to make sure the lint result is clean before we commit this, right? support/mesos-style.py Lines 332-334 (patched) <https://reviews.apache.org/r/62214/#comment261332> Copy paste? support/mesos-style.py Lines 344-361 (patched) <https://reviews.apache.org/r/62214/#comment261333> Can you document that we build the virtualenv by running bootstrap? It will be unfortunate to have the code here and in PyLinter diverge, have you considered adding some kind of VirtualEnv class or making these standalone virtualenv functions here for them to both reuse? support/mesos-style.py Lines 343-345 (original) <https://reviews.apache.org/r/62214/#comment261330> What happened here? Should this be a different patch? - Benjamin Mahler On Sept. 11, 2017, 9:49 a.m., Armand Grillet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62214/ > ----------------------------------------------------------- > > (Updated Sept. 11, 2017, 9:49 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-7924 > https://issues.apache.org/jira/browse/MESOS-7924 > > > Repository: mesos > > > Description > ------- > > This linter runs when changes on a JavaScript file are being committed. > The linter used is ESLint with a configuration based on our current JS > code base. The linter and its dependencies (i.e. Node.js) are installed > in a virtual environment using Virtualenv and then Nodeenv. > > > Diffs > ----- > > src/.gitignore PRE-CREATION > src/webui/master/static/.eslintrc.js PRE-CREATION > src/webui/master/static/.gitignore PRE-CREATION > src/webui/master/static/bootstrap PRE-CREATION > src/webui/master/static/pip-requirements.txt PRE-CREATION > support/mesos-style.py cf37d9f4da4ab90b92f0136a1dcd5dd8bbae5785 > > > Diff: https://reviews.apache.org/r/62214/diff/1/ > > > Testing > ------- > > Following this commit, I have tried to commit a change on a JavaScript file > and checked that ESLinter was correctly invoked. > > > Thanks, > > Armand Grillet > >
