You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/10/20 21:19:08 UTC
[GitHub] [airflow] ryanahamilton opened a new pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
ryanahamilton opened a new pull request #11699:
URL: https://github.com/apache/airflow/pull/11699
The codebase has existing ESLint JavaScript linting rules configured, but they are not enforced (until now).
- I've added the `eslint` check to the pre-commit hooks.
- Refactored all of the existing files to be compliant with the rules so checks are passing.
- This is only being ran against external files (for now), as some additional refactoring will be required to get the inline scripting in order as well.
- I manually tested all of the webserver views that utilize these files and was unable to yield any console errors.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713703059
[The Workflow run](https://github.com/apache/airflow/actions/runs/320195050) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-721161003
[The Workflow run](https://github.com/apache/airflow/actions/runs/343775994) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-714428227
[The Workflow run](https://github.com/apache/airflow/actions/runs/321920699) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713152550
Does this eslint configuration meet the project style guide?
https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#follow-javascript-style-guide
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-720937313
[The Workflow run](https://github.com/apache/airflow/actions/runs/343086174) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-721344784
[The Workflow run](https://github.com/apache/airflow/actions/runs/344267994) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] ryanahamilton commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713154903
@mik-laj yes this is running the same configuration as `yarn run lint`, it just has a narrower scope (doesn't run against HTML files yet).
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#discussion_r508854034
##########
File path: airflow/www/static/js/base.js
##########
@@ -47,57 +48,56 @@ function changDisplayedTimezone(tz) {
});
}
-var el = document.createElement('span');
-
export function escapeHtml(text) {
+ const el = document.createElement('span');
el.textContent = text;
return el.innerHTML;
}
window.escapeHtml = escapeHtml;
-export function convertSecsToHumanReadable(seconds) {
- var oriSeconds = seconds;
- var floatingPart = oriSeconds- Math.floor(oriSeconds);
+export function convertSecsToHumanReadable(secs) {
+ const oriSeconds = secs;
Review comment:
Do we have Babel on? I know there were some compatibility issues with older browsers.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-721199464
[The Workflow run](https://github.com/apache/airflow/actions/runs/343847050) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-714491956
[The Workflow run](https://github.com/apache/airflow/actions/runs/322120249) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-714615190
[The Workflow run](https://github.com/apache/airflow/actions/runs/322423092) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-714413097
Rebased as Master was failing
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713165356
[The Workflow run](https://github.com/apache/airflow/actions/runs/318606200) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713899285
[The Workflow run](https://github.com/apache/airflow/actions/runs/320797006) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks$,^Build docs$,^Spell check docs$,^Backport packages$,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] ryanahamilton commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713535462
@ashb pre-commit wasn't including files outside of the `/js/` directory locally—I think I just need to add an explicit argument to restrict to that directory—though it might be preferred to do as you suggest and check/fix all of the files?
----------------------------------------------------------------
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
[GitHub] [airflow] mik-laj commented on a change in pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#discussion_r508854034
##########
File path: airflow/www/static/js/base.js
##########
@@ -47,57 +48,56 @@ function changDisplayedTimezone(tz) {
});
}
-var el = document.createElement('span');
-
export function escapeHtml(text) {
+ const el = document.createElement('span');
el.textContent = text;
return el.innerHTML;
}
window.escapeHtml = escapeHtml;
-export function convertSecsToHumanReadable(seconds) {
- var oriSeconds = seconds;
- var floatingPart = oriSeconds- Math.floor(oriSeconds);
+export function convertSecsToHumanReadable(secs) {
+ const oriSeconds = secs;
Review comment:
Do we have Babel? I know there were some compatibility issues with older browsers.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-721292129
[The Workflow run](https://github.com/apache/airflow/actions/runs/344114915) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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
[GitHub] [airflow] potiuk commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-713539347
> @ashb pre-commit wasn't including files outside of the `/js/` directory locally—I think I just need to add an explicit argument to restrict to that directory—though it might be preferred to do as you suggest and check/fix all of the files?
Yeah - we are running pre-commit with --all-files switch in CI.
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-714529978
@ryanahamilton CI failed with the following:
```
ESLint............................................................................................................................Failed
- hook id: eslint
- exit code: 1
Error resolving webpackConfig Error: Cannot find module 'webpack'
Require stack:
- /home/runner/work/airflow/airflow/ai
.....
```
https://github.com/apache/airflow/pull/11699/checks?check_run_id=1292691649#step:7:176
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-787558826
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
----------------------------------------------------------------
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
[GitHub] [airflow] github-actions[bot] closed pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #11699:
URL: https://github.com/apache/airflow/pull/11699
----------------------------------------------------------------
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
[GitHub] [airflow] kaxil commented on pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#issuecomment-721445214
Can you please rebase your PR on latest Master since we have applied [Black](https://github.com/apache/airflow/commit/4e8f9cc8d02b29c325b8a5a76b4837671bdf5f68) and [PyUpgrade](https://github.com/apache/airflow/commit/8c42cf1b00c90f0d7f11b8a3a455381de8e003c5) on Master.
It will help if your squash your commits into single commit first so that there are less conflicts.
----------------------------------------------------------------
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
[GitHub] [airflow] ryanahamilton commented on a change in pull request #11699: Enforce ESLint (JavaScript linting) with pre-commit hooks
Posted by GitBox <gi...@apache.org>.
ryanahamilton commented on a change in pull request #11699:
URL: https://github.com/apache/airflow/pull/11699#discussion_r508846727
##########
File path: airflow/www/templates/airflow/dags.html
##########
@@ -75,7 +75,7 @@ <h2>DAGs</h2>
<thead>
<tr>
<th width="12">
- <span id="pause_header" class="material-icons text-muted" title="Use this toggle to pause a DAG. The scheduler won't schedule new tasks instances for a paused DAG. Tasks already running at pause time won't be affected.">info</span>
+ <span class="material-icons text-muted js-tooltip" title="Use this toggle to pause a DAG. The scheduler won't schedule new tasks instances for a paused DAG. Tasks already running at pause time won't be affected.">info</span>
Review comment:
This change is unrelated to this PR, but fixes a scripting interaction I discovered during my manual testing.
----------------------------------------------------------------
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