You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "josh-fell (via GitHub)" <gi...@apache.org> on 2024/01/04 16:28:51 UTC

[PR] Add flake8-implicit-str-concat check to Ruff [airflow]

josh-fell opened a new pull request, #36597:
URL: https://github.com/apache/airflow/pull/36597

   This was enabled initially in #23873, but hasn't been part of the Ruff checks to-date. Let's add it!
   


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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36597:
URL: https://github.com/apache/airflow/pull/36597#issuecomment-1877590649

   > Other than one comment, looks good to me! Love this one ❤️
   
   Likewise :)


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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36597:
URL: https://github.com/apache/airflow/pull/36597#issuecomment-1877624160

   > nice, just wondering how those string concatenations existed in so many places :) maybe it was another pre-commit responsible for it.
   
   We used to get rid of them by flake I think - but at the time we moved to ruff it was not available as far as I remember. 
   And those strings are create by black reformatting - for example when you remove and the whole block gets de-dented previously split strings (because they were too long) are moved to one line. But black (and ruff-black follows it) has the rule that it never EVER touches the AST so concatenating two strings is not possible in ruff.
   
   That's the root cause we had so many of those.
   
   And yes. I got far too many of those as well and it started to be annoying :) 


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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell merged PR #36597:
URL: https://github.com/apache/airflow/pull/36597


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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #36597:
URL: https://github.com/apache/airflow/pull/36597#issuecomment-1877532434

   Other than one comment, looks good to me! Love this one :heart: 


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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #36597:
URL: https://github.com/apache/airflow/pull/36597#discussion_r1442121987


##########
pyproject.toml:
##########
@@ -188,3 +189,8 @@ exclude_also = [
     "@(typing(_extensions)?\\.)?overload",
     "if (typing(_extensions)?\\.)?TYPE_CHECKING:"
 ]
+
+[tool.vulture]
+min_confidence = 80
+paths = ["airflow/"]
+sort_by_size = true

Review Comment:
   Oops, got code bleed from something else I was working on. Thanks for catching!



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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "josh-fell (via GitHub)" <gi...@apache.org>.
josh-fell commented on code in PR #36597:
URL: https://github.com/apache/airflow/pull/36597#discussion_r1442122727


##########
pyproject.toml:
##########
@@ -188,3 +189,8 @@ exclude_also = [
     "@(typing(_extensions)?\\.)?overload",
     "if (typing(_extensions)?\\.)?TYPE_CHECKING:"
 ]
+
+[tool.vulture]
+min_confidence = 80
+paths = ["airflow/"]
+sort_by_size = true

Review Comment:
   Removed now.



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


Re: [PR] Add flake8-implicit-str-concat check to Ruff [airflow]

Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #36597:
URL: https://github.com/apache/airflow/pull/36597#discussion_r1442091359


##########
pyproject.toml:
##########
@@ -188,3 +189,8 @@ exclude_also = [
     "@(typing(_extensions)?\\.)?overload",
     "if (typing(_extensions)?\\.)?TYPE_CHECKING:"
 ]
+
+[tool.vulture]
+min_confidence = 80
+paths = ["airflow/"]
+sort_by_size = true

Review Comment:
   I didn't expect to see two changes to this file, what does this one do?



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