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/05/27 03:43:22 UTC

[GitHub] [airflow] kaxil commented on a change in pull request #9010: Move setup order check back to pre-commit

kaxil commented on a change in pull request #9010:
URL: https://github.com/apache/airflow/pull/9010#discussion_r430053355



##########
File path: dev/check_order_setup.py
##########
@@ -0,0 +1,175 @@
+#!/usr/bin/env python

Review comment:
       Why did we change the location of this file?

##########
File path: dev/check_order_setup.py
##########
@@ -0,0 +1,175 @@
+#!/usr/bin/env python

Review comment:
       I would probably in `scripts/ci` where we already have other scripts for precommits like others, Example:
   
   ```
   ❯ ls -ltr scripts/ci | grep pre_commit
   -rwxr-xr-x@  1 kaxilnaik  staff   1105 18 May 20:12 pre_commit_bat_tests.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1958 18 May 20:12 pre_commit_breeze_cmd_line.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1136 18 May 20:12 pre_commit_check_license.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1062 18 May 20:12 pre_commit_ci_build.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1012 18 May 20:12 pre_commit_flake8.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1077 18 May 20:12 pre_commit_generate_requirements.sh
   -rwxr-xr-x@  1 kaxilnaik  staff    960 18 May 20:12 pre_commit_lint_dockerfile.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1808 18 May 20:12 pre_commit_local_yml_mounts.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1008 18 May 20:12 pre_commit_mypy.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1088 18 May 20:12 pre_commit_update_extras.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1474 21 May 17:06 pre_commit_check_integrations.sh
   -rwxr-xr-x   1 kaxilnaik  staff   5321 23 May 02:17 pre_commit_yaml_to_cfg.py
   ```
   
   . Things inside `dev` aren't really tested and currently mostly only contains Release related files

##########
File path: dev/check_order_setup.py
##########
@@ -0,0 +1,175 @@
+#!/usr/bin/env python

Review comment:
       I would probably say, let's use `scripts/ci` folder where we already have other scripts for precommits like others, Example:
   
   ```
   ❯ ls -ltr scripts/ci | grep pre_commit
   -rwxr-xr-x@  1 kaxilnaik  staff   1105 18 May 20:12 pre_commit_bat_tests.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1958 18 May 20:12 pre_commit_breeze_cmd_line.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1136 18 May 20:12 pre_commit_check_license.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1062 18 May 20:12 pre_commit_ci_build.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1012 18 May 20:12 pre_commit_flake8.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1077 18 May 20:12 pre_commit_generate_requirements.sh
   -rwxr-xr-x@  1 kaxilnaik  staff    960 18 May 20:12 pre_commit_lint_dockerfile.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1808 18 May 20:12 pre_commit_local_yml_mounts.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1008 18 May 20:12 pre_commit_mypy.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1088 18 May 20:12 pre_commit_update_extras.sh
   -rwxr-xr-x@  1 kaxilnaik  staff   1474 21 May 17:06 pre_commit_check_integrations.sh
   -rwxr-xr-x   1 kaxilnaik  staff   5321 23 May 02:17 pre_commit_yaml_to_cfg.py
   ```
   
   . Things inside `dev` aren't really tested and currently mostly only contains Release related files

##########
File path: dev/check_order_setup.py
##########
@@ -0,0 +1,175 @@
+#!/usr/bin/env python

Review comment:
       LGTM other than that




----------------------------------------------------------------
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