You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2019/08/20 05:49:00 UTC

[jira] [Commented] (AIRFLOW-5264) Fix all linting errors in .js files

    [ https://issues.apache.org/jira/browse/AIRFLOW-5264?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16911016#comment-16911016 ] 

ASF GitHub Bot commented on AIRFLOW-5264:
-----------------------------------------

potiuk commented on pull request #5871:  [AIRFLOW-5264] Eslint + npmaudit (depends on  [AIRFLOW-5247])
URL: https://github.com/apache/airflow/pull/5871
 
 
   NOTE! This PR depends on AIRFLOW-5247 as it requires NPM installed in static check dockerfile. So please check only the last commit.
   
   This is (initially experimental) change that introduces ESLiniting and npm audit as pre-commit hooks. Introduction of those two required fixing all current ESLint errors and upgrading NPM dependencies to latest versions (there were more than 50 vulnerabilities reported via npm audit with the current master version of the JS libraries).
   
   It was initially experimental, because I was not sure whether this will be successful. I upgraded to latest version of dependencies using npm audit and then upgraded to latest version of eslint and finally run `eslint --fix`. Surprisingly there were just a few errors left that were easy to fix manually. Also when I run the webserver - going through pretty much all the screens I did not see any errors.
   
   Of course it requires quite a bit more testing but I think if we make a small group effort to test it, we might get rid of all the ESLint problems and thanks to pre-commit hooks also keep it like that in the future. 
   
   I am aware of Closed  #3656 and the idea from @mik-laj about separating out the HTML code to .js - and finally https://issues.apache.org/jira/browse/AIRFLOW-2803 which I created subtask of. However I think if we merge this one - this will be great step forward with the .js code improvement. Having eslint + npm audit in place is already great - especially from security point of view.
   
   Note - I added npm audit as separate pre-commit hook, in the way that if someone changes any .js code, it will be run locally (and potentially point out to some upgrades that should be performed) but I skipped it in regular CI builds so that audit problems will not block us from merging unrelated changes. I run it however in the daily CRON build - so that we can get an early warning that some NPM dependency requires  upgrade due to security vulnerability.
   
   ### Jira
   
   - [x] My PR addresses the following [Airflow Jira](https://issues.apache.org/jira/browse/AIRFLOW/) issues and references them in the PR title. For example, "\[AIRFLOW-XXX\] My Airflow PR"
     - https://issues.apache.org/jira/browse/AIRFLOW-5264
   
   ### Description
   
   - [x] Here are some details about my PR, including screenshots of any UI changes:
   
   ### Tests
   
   - [x] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
   
   ### Commits
   
   - [x] My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
     1. Subject is separated from body by a blank line
     1. Subject is limited to 50 characters (not including Jira issue reference)
     1. Subject does not end with a period
     1. Subject uses the imperative mood ("add", not "adding")
     1. Body wraps at 72 characters
     1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [x] In case of new functionality, my PR adds documentation that describes how to use it.
     - All the public functions and the classes in the PR contain docstrings that explain what it does
     - If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release
   
   ### Code Quality
   
   - [x] Passes `flake8`
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Fix all linting errors in .js files
> -----------------------------------
>
>                 Key: AIRFLOW-5264
>                 URL: https://issues.apache.org/jira/browse/AIRFLOW-5264
>             Project: Apache Airflow
>          Issue Type: Sub-task
>          Components: webserver
>    Affects Versions: 2.0.0
>            Reporter: Jarek Potiuk
>            Priority: Major
>




--
This message was sent by Atlassian Jira
(v8.3.2#803003)