Skip to content

Google provider December removal#145

Closed
olegkachur-e wants to merge 2 commits into
mainfrom
googe_provider_december_removal
Closed

Google provider December removal#145
olegkachur-e wants to merge 2 commits into
mainfrom
googe_provider_december_removal

Conversation

@olegkachur-e
Copy link
Copy Markdown
Collaborator


^ 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.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Copy Markdown

@moiseenkov moiseenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!
There are only two things I'd kindly ask you to amend:

  1. please add an announcement about breaking changes and the major version bump in the providers/src/airflow/providers/google/CHANGELOG.rst the same way it has been done last time.
  2. please rename unit test classes, so the old operators' names were vanished from everywhere and the current tests had consistent names:
    TestCreateDataPipelineOperator -> TestDataflowCreatePipelineOperator
    TestRunDataPipelineOperator -> TestDataflowRunPipelineOperator

@olegkachur-e olegkachur-e force-pushed the googe_provider_december_removal branch from a3e3aee to 91f5644 Compare December 19, 2024 12:18
@olegkachur-e
Copy link
Copy Markdown
Collaborator Author

Well done! There are only two things I'd kindly ask you to amend:

  1. please add an announcement about breaking changes and the major version bump in the providers/src/airflow/providers/google/CHANGELOG.rst the same way it has been done last time.
  2. please rename unit test classes, so the old operators' names were vanished from everywhere and the current tests had consistent names:
    TestCreateDataPipelineOperator -> TestDataflowCreatePipelineOperator
    TestRunDataPipelineOperator -> TestDataflowRunPipelineOperator

Brilliant mentions, thank you!

@moiseenkov
Copy link
Copy Markdown

Well done! There are only two things I'd kindly ask you to amend:

  1. please add an announcement about breaking changes and the major version bump in the providers/src/airflow/providers/google/CHANGELOG.rst the same way it has been done last time.
  2. please rename unit test classes, so the old operators' names were vanished from everywhere and the current tests had consistent names:
    TestCreateDataPipelineOperator -> TestDataflowCreatePipelineOperator
    TestRunDataPipelineOperator -> TestDataflowRunPipelineOperator

Brilliant mentions, thank you!

Thank you! All is good, but please move changelog announcements into a new section for the new 12.0.0 version.

@olegkachur-e olegkachur-e force-pushed the googe_provider_december_removal branch from 91f5644 to 37c47e8 Compare December 19, 2024 14:30
Comment thread providers/src/airflow/providers/google/CHANGELOG.rst Outdated
@olegkachur-e olegkachur-e force-pushed the googe_provider_december_removal branch from 37c47e8 to dfb7096 Compare December 19, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants