-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fail PR when common compat changes and there is no # use next version #58840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc: @ephraimbuddy :) -> this way in the future, the one who modifies common.compat will not be able to merge their PR until |
da2d7dd to
90e24d7
Compare
jscheffl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of AI generated code that is very verbose. Sometimes I think the AI bot is paid by LoC generated and not by making it conceise.
90e24d7 to
ae8b0d5
Compare
Yeah. I did it late in night and after waking up, it was a bit too verbose. I asked Claude Sonnet to simplify and it not only did all the changes you suggested but also nicely refactored it into separate private methods which IMHO makes it far easier to comprehend and understand what we are doing |
ae8b0d5 to
967389a
Compare
When common.compat changes, but the providers that use it, do not have `# use next version` added for the common-compat dependency, the PR should fail. You can still bypass the check with the label set on the PR: 'skip common compat check'
967389a to
f1235e6
Compare
Not in this case, but in a way that might be a good thing. More verbose code is quite often easier to understand - even if longer to read. Sometimes the comprehensions take quite a bit of mental effort to understand where simple for loop is clear and straightforrward. I think human authors (comparing to AI) over-optimise for "writing" rather than "reading" - and writing concise and short code is preferred by some people, but it only works to a point. But yes. This one was definitely too chatty. |
Backport failed to create: v3-1-test. View the failure log Run details
You can attempt to backport this manually by running: cherry_picker 65aec2d v3-1-testThis should apply the commit to the v3-1-test branch and leave the commit in conflict state marking After you have resolved the conflicts, you can continue the backport process by running: cherry_picker --continue |
…apache#58840) When common.compat changes, but the providers that use it, do not have `# use next version` added for the common-compat dependency, the PR should fail. You can still bypass the check with the label set on the PR: 'skip common compat check'
…apache#58840) When common.compat changes, but the providers that use it, do not have `# use next version` added for the common-compat dependency, the PR should fail. You can still bypass the check with the label set on the PR: 'skip common compat check'
…apache#58840) When common.compat changes, but the providers that use it, do not have `# use next version` added for the common-compat dependency, the PR should fail. You can still bypass the check with the label set on the PR: 'skip common compat check'

When common.compat changes, but the providers that use it, do not have
# use next versionadded for the common-compat dependency, the PR should fail.You can still bypass the check with the label set on the PR:
'skip common compat check'
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.