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 2021/03/16 15:59:10 UTC
[GitHub] [airflow] bbovenzi opened a new pull request #14836: Pre commit new UI
bbovenzi opened a new pull request #14836:
URL: https://github.com/apache/airflow/pull/14836
Draft PR for adding pre-commit hooks to lint and test the new UI.
---
**^ 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 #14836: Pre commit new UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14836:
URL: https://github.com/apache/airflow/pull/14836#issuecomment-800605189
[The Workflow run](https://github.com/apache/airflow/actions/runs/658979462) 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 a change in pull request #14836: Pre commit new UI
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14836:
URL: https://github.com/apache/airflow/pull/14836#discussion_r595416991
##########
File path: .pre-commit-config.yaml
##########
@@ -563,6 +563,10 @@ repos:
language_version: python3
additional_dependencies: ['flynt']
files: \.py$
+ - id: ui-lint-test
+ name: Lint and test React UI
+ language: system
+ entry: ./scripts/ci/pre_commit/pre_commit_ui_lint_test.sh
Review comment:
I believe using system hook and script is not needed (but I am not sure if it works with yarn)
pre-commit has a built-in `language: node` and it understand package.json. Not sure about yarn.lock though . But if we can make it works - it will even automatically rebuild the node environment for you and allow to run yarn commands.
You can some examples here:
https://github.com/Exocortex/pre-commit/blob/master/.pre-commit-hooks.yaml
Also I think you should select the files that the lint should run on - .js files basically. Pre-commit has the nice feature that it only runs the checks on files that are changed in your commit but also splits the checks to N parallell checks - as many as many processors you have. So for sure you need "files:" field to tell pre-commit which files it should run on and your command (either script or yarn command) should take those files as parameters.
For example when you only change a.js, b.js, c.js but not d.js and you have 2 CPUs precommit should run two commands in parallel:
```
yarn lint a.js b.js
yarn lint c.js
```
----------------------------------------------------------------
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 #14836: Pre commit new UI
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14836:
URL: https://github.com/apache/airflow/pull/14836#issuecomment-800602806
[The Workflow run](https://github.com/apache/airflow/actions/runs/658961439) 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 merged pull request #14836: Pre commit new UI
Posted by GitBox <gi...@apache.org>.
ryanahamilton merged pull request #14836:
URL: https://github.com/apache/airflow/pull/14836
----------------------------------------------------------------
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] bbovenzi commented on a change in pull request #14836: Pre commit new UI
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #14836:
URL: https://github.com/apache/airflow/pull/14836#discussion_r595429536
##########
File path: .pre-commit-config.yaml
##########
@@ -563,6 +563,10 @@ repos:
language_version: python3
additional_dependencies: ['flynt']
files: \.py$
+ - id: ui-lint-test
+ name: Lint and test React UI
+ language: system
+ entry: ./scripts/ci/pre_commit/pre_commit_ui_lint_test.sh
Review comment:
Good call. Updated. I also made `lint` and `test` separate hooks.
----------------------------------------------------------------
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