mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Armand Grillet <agril...@mesosphere.io>
Subject Re: Review Request 62214: Added JavaScript linter.
Date Fri, 15 Sep 2017 10:30:17 GMT


> On Sept. 11, 2017, 7:18 p.m., Benjamin Mahler wrote:
> > support/mesos-style.py
> > Lines 307-311 (patched)
> > <https://reviews.apache.org/r/62214/diff/1/?file=1819479#file1819479line307>
> >
> >     Similarly to my comment below, if we had some virtual env abstraction this could
be running something within in:
> >     
> >     ```
> >     virtualenv.run_within(['eslint', ...]);
> >     ```

The `run_lint()` functions are quite different depending on the linter and only the activation
of the virtual environment is done in both functions for the `.js` and `.py` linters. I could
still had a `main()` function in /r/62333 if you think that is necessary.


> On Sept. 11, 2017, 7:18 p.m., Benjamin Mahler wrote:
> > support/mesos-style.py
> > Lines 332-334 (patched)
> > <https://reviews.apache.org/r/62214/diff/1/?file=1819479#file1819479line332>
> >
> >     Copy paste?

We have a `pip-requirements` to install nodeenv.


> On Sept. 11, 2017, 7:18 p.m., Benjamin Mahler wrote:
> > support/mesos-style.py
> > Lines 344-361 (patched)
> > <https://reviews.apache.org/r/62214/diff/1/?file=1819479#file1819479line344>
> >
> >     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?

/r/62333/


- Armand


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


On Sept. 14, 2017, 3:59 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> -----------------------------------------------------------
> 
> (Updated Sept. 14, 2017, 3:59 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 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 environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -----
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py cf37d9f4da4ab90b92f0136a1dcd5dd8bbae5785 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/2/
> 
> 
> Testing
> -------
> 
> Following this commit, I have tried to commit a change on a JavaScript file and checked
that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


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