[GitHub] [airflow] potiuk commented on a change in pull request #16658: maintained standards of formatting and annotation

[GitBox <gi...@apache.org>]

potiuk commented on a change in pull request #16658:
URL: https://github.com/apache/airflow/pull/16658#discussion_r658889352



##########
File path: setup.py
##########
@@ -62,22 +62,22 @@ class CleanCommand(Command):
     description = "Tidy up the project root"
     user_options: List[str] = []
 
-    def initialize_options(self):
+    def initialize_options(self) -> None:
         """Set default values for options."""
 
-    def finalize_options(self):
+    def finalize_options(self) -> None:
         """Set final values for options."""
 
     @staticmethod
-    def rm_all_files(files: List[str]):
+    def rm_all_files(files: List[str]) -> None:
         """Remove all files from the list"""
         for file in files:
             try:
                 os.remove(file)
             except Exception as e:  # noqa pylint: disable=broad-except
-                logger.warning("Error when removing %s: %s", file, e)
+                logger.warning(f"Error when removing {file}: {e}")

Review comment:
       Using F-strings for logger is bad-practice, please revert that. Read for example here why: 
   
   https://www.siegescape.com/blog/2019/10/3/be-careful-with-your-f-strings

##########
File path: setup.py
##########
@@ -833,7 +833,7 @@ def get_provider_package_from_package_id(package_id: str):
     return f"apache-airflow-providers-{package_suffix}"
 
 
-def get_excluded_providers():
+def get_excluded_providers() -> List:

Review comment:
       ```suggestion
   def get_excluded_providers() -> List[str]:
   ```




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

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org