Skip to content
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

✨ Bump Gophercloud to v2 beta #1875

Closed
wants to merge 1 commit into from
Closed

Conversation

EmilienM
Copy link
Contributor

@EmilienM EmilienM commented Feb 9, 2024

What this PR does / why we need it:
Update Gophercloud + Utils to v2-beta

/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 9, 2024
Copy link

netlify bot commented Feb 9, 2024

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
🔨 Latest commit 6f57168
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/65e208fffeca450008ac3b0e
😎 Deploy Preview https://deploy-preview-1875--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 9, 2024
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 9, 2024
go.mod Outdated Show resolved Hide resolved
@EmilienM EmilienM changed the title WIP - gophercloud v2 beta WIP - bump gophercloud to v2 beta Feb 9, 2024
go.mod Outdated Show resolved Hide resolved
@EmilienM EmilienM changed the title WIP - bump gophercloud to v2 beta Bump Gophercloud to v2 beta Feb 14, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2024
@EmilienM EmilienM changed the title Bump Gophercloud to v2 beta ✨ Bump Gophercloud to v2 beta Feb 14, 2024
@EmilienM
Copy link
Contributor Author

This is ready for review.

Copy link
Contributor

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Awesome!
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EmilienM, lentzi90

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2024
@huxcrux
Copy link
Contributor

huxcrux commented Feb 15, 2024

Sweet to get all new fixes :)
/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 15, 2024
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 15, 2024
@pierreprinetti
Copy link
Contributor

Please note that we are likely to change all function signatures in v2-beta.2 to include a context.Context.

@mdbooth
Copy link
Contributor

mdbooth commented Feb 16, 2024

Please note that we are likely to change all function signatures in v2-beta.2 to include a context.Context.

Very much in favour of landing this once v2 lands, but not before, for example because of the possibility of changes like the above prior to release.

I think gophercloud is pretty close to releasing that though, right?

@jichenjc
Copy link
Contributor

Update Gophercloud + Utils to v2-beta
I think gophercloud is pretty close to releasing that though, right?

also curious about when v2 will land ,something like CPO will benefit as well if some enhancement can be adopted

@pierreprinetti
Copy link
Contributor

I think gophercloud is pretty close to releasing that though, right?

Yes although I'd measure the ETA in weeks rather than days or hours.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 22, 2024
@EmilienM
Copy link
Contributor Author

@pierreprinetti sounds like we could update this one to use beta-2 but this will need a bunch of changes for ctx, right?

@pierreprinetti
Copy link
Contributor

@pierreprinetti sounds like we could update this one to use beta-2 but this will need a bunch of changes for ctx, right?

correct beta.2 adds ctx everywhere

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 1, 2024
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 1, 2024
@pierreprinetti
Copy link
Contributor

Updated to v2.0.0-beta.2. Calls to Gophercloud now use context.TODO(). For now I am deferring the decision whether to actually use the context in the whole CAPO.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 1, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 1, 2024
Co-Authored-By: Emilien Macchi <emacchi@redhat.com>
Co-Authored-By: Pierre Prinetti <pierreprinetti@redhat.com>
@huxcrux
Copy link
Contributor

huxcrux commented Mar 1, 2024

/test pull-cluster-api-provider-openstack-e2e-test

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 5, 2024
@k8s-ci-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@EmilienM
Copy link
Contributor Author

#2107

@EmilienM EmilienM closed this May 31, 2024
@mdbooth mdbooth deleted the gv2 branch May 31, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

9 participants