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/06/09 16:12:01 UTC

[GitHub] [airflow] feluelle opened a new pull request #9195: Update pre-commit-hooks repo version

feluelle opened a new pull request #9195:
URL: https://github.com/apache/airflow/pull/9195


   - use official isort pre-commit-hook
   - use official yamllint pre-commit-hook
   
   ---
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Target Github ISSUE in description if exists
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [x] Relevant documentation is updated including usage instructions.
   - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   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).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   


----------------------------------------------------------------
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] feluelle commented on pull request #9195: Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9195:
URL: https://github.com/apache/airflow/pull/9195#issuecomment-641870813


   Thanks all - will fix the issues.


----------------------------------------------------------------
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] ashb commented on pull request #9195: Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #9195:
URL: https://github.com/apache/airflow/pull/9195#issuecomment-641549005


   isort seems to be wreaking havok with the UPDATING.md -- we might have to not pick the latest version (or else turn it tune it down):
   
   ```diff
   diff --git a/docs/howto/operator/gcp/gcs_to_gcs.rst b/docs/howto/operator/gcp/gcs_to_gcs.rst
   index e17bedf..bb3fda1 100644
   --- a/docs/howto/operator/gcp/gcs_to_gcs.rst
   +++ b/docs/howto/operator/gcp/gcs_to_gcs.rst
   @@ -40,11 +40,9 @@ perform this task faster and more economically. The economic effects are especia
    Airflow is not hosted in Google Cloud Platform, because these operators reduce egress traffic.
    
    These operators modify source objects if the option that specifies whether objects should be deleted
   -from the source after they are transferred to the sink is enabled.
    
    When you use the Google Cloud Data Transfer service, you can specify whether overwriting objects that already exist in
    the sink is allowed, whether objects that exist only in the sink should be deleted, or whether objects should be deleted
   -from the source after they are transferred to the sink.
    
    Source objects can be specified using include and exclusion prefixes, as well as based on the file
    modification date.
   ```
   
   It seems to think _any_ line starting with `from` is an import line! We need to only have it operate on .py 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] feluelle edited a comment on pull request #9195: [WIP] Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
feluelle edited a comment on pull request #9195:
URL: https://github.com/apache/airflow/pull/9195#issuecomment-642473375


   > It seems to think any line starting with from is an import line! We need to only have it operate on .py files
   
   [4.3.21](https://github.com/timothycrosley/isort/blob/4.3.21-2/.pre-commit-hooks.yaml) fixed it.
   [4.3.21-2](https://github.com/timothycrosley/isort/blob/4.3.21-2/.pre-commit-hooks.yaml) broke it.
   [develop](https://github.com/timothycrosley/isort/blob/develop/.pre-commit-hooks.yaml) has the correct version, but it is not released yet. So I added it here and use `4.3.21-2` so we have the latest version of isort.


----------------------------------------------------------------
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 merged pull request #9195: Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #9195:
URL: https://github.com/apache/airflow/pull/9195


   


----------------------------------------------------------------
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] feluelle commented on pull request #9195: [WIP] Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9195:
URL: https://github.com/apache/airflow/pull/9195#issuecomment-642473375


   > It seems to think any line starting with from is an import line! We need to only have it operate on .py files
   
   This is fixed in isort `develop` but not yet released :/ So I added it here.


----------------------------------------------------------------
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 #9195: Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9195:
URL: https://github.com/apache/airflow/pull/9195#issuecomment-641418199


   It looks like the new stylelint requires some style improvements as well @feluelle 


----------------------------------------------------------------
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] feluelle commented on a change in pull request #9195: Update pre-commit-hooks repo version

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9195:
URL: https://github.com/apache/airflow/pull/9195#discussion_r437508400



##########
File path: .pre-commit-config.yaml
##########
@@ -159,15 +159,23 @@ metastore_browser/templates/.*\\.html$|.*\\.jinja2"
     hooks:
       - id: rst-backticks
       - id: python-no-log-warn
-  - repo: local
+  - repo: https://github.com/adrienverge/yamllint
+    rev: v1.23.0

Review comment:
       Fix version - I set it to the latest version.

##########
File path: .pre-commit-config.yaml
##########
@@ -141,7 +141,7 @@ metastore_browser/templates/.*\\.html$|.*\\.jinja2"
     hooks:
       - id: check-hooks-apply
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v2.5.0
+    rev: v3.1.0

Review comment:
       > pre-commit/pre-commit-hooks now requires python3.6.1+
   
   That is the only noticeable difference for us.
   
   Ref: https://github.com/pre-commit/pre-commit-hooks/releases

##########
File path: .pre-commit-config.yaml
##########
@@ -159,15 +159,23 @@ metastore_browser/templates/.*\\.html$|.*\\.jinja2"
     hooks:
       - id: rst-backticks
       - id: python-no-log-warn
-  - repo: local
+  - repo: https://github.com/adrienverge/yamllint
+    rev: v1.23.0
     hooks:
       - id: yamllint
         name: Check yaml files with yamllint
         entry: yamllint -c yamllint-config.yml
-        language: python
-        additional_dependencies: ['yamllint']
         types: [yaml]
         exclude: ^.*init_git_sync\.template\.yaml$|^.*airflow\.template\.yaml$
+  - repo: https://github.com/timothycrosley/isort
+    rev: 4.3.21-2

Review comment:
       Fix version - I set it to the latest version.




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