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 2019/09/03 13:03:24 UTC

[GitHub] [airflow] potiuk commented on a change in pull request #5944: [AIRFLOW-5362] Reorder imports

potiuk commented on a change in pull request #5944: [AIRFLOW-5362] Reorder imports
URL: https://github.com/apache/airflow/pull/5944#discussion_r320259098
 
 

 ##########
 File path: airflow/_vendor/nvd3/__init__.py
 ##########
 @@ -16,14 +16,14 @@
            'scatterChart', 'discreteBarChart', 'multiBarChart']
 
 
+from . import ipynb
+from .cumulativeLineChart import cumulativeLineChart
 
 Review comment:
   I have another thought. 
   
   I think one big change to order all imports in one go is not a good idea. We should probably use the same approach as we did for pylint.
   
   Since we can have all of this as pre-commit hooks - we could do filtering in the same way we do for pylint pre-commit hooks/ci_scripts using the very same "pylint_todo.txt" file. 
   I think those two are fairly connected and we should use the list to only introduce such changes when we remove files from the pylint_todo.txt. And this way we make sure it will be gradually introduced.
    
   Once we add it as pre-commit hook, it will have the property that someone fixing pylint will also have to make sure that imports are sorted. I also think about adding pydocstyle in exactly the same way -> so make sure the style is defined and checked for all files that are not on the pylint_todo.txt. This way reviewers will not have to even check if the documentation style we have is good and follows our agreed conventions. I think this should be automated rather than taking precious time of reviewers :).
   
   What do you think (@KevinYang21 @BasPH @ashb @mik-laj ) ? I can add it quickly while I will be adding pydocstyle (I plan to test it while fixing @mik-laj comments for #5786) and I can add isort in very similar way there.

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


With regards,
Apache Git Services