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

Upgrading to v16.2.1 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest #1198

Merged
merged 2 commits into from
May 7, 2018

Conversation

tombuildsstuff
Copy link
Contributor

@tombuildsstuff tombuildsstuff commented May 3, 2018

Upgrading to the most recent versions of the SDK:

  • v16.2.1 of Azure/azure-sdk-for-go
  • v10.8.1 of Azure/go-autorest

@tombuildsstuff tombuildsstuff added this to the 1.5.0 milestone May 3, 2018
@metacpp
Copy link
Contributor

metacpp commented May 3, 2018

@tombuildsstuff what's the motivation to upgrade to 16.01? This change will definitely conflict with Jeffrey's 2 pending PRs(15.x), Junyi's PR(15.x), my PR(required 16.2).

@metacpp metacpp self-requested a review May 3, 2018 21:22
@tombuildsstuff tombuildsstuff changed the title [WIP] Upgrading to v16.0.1 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest Upgrading to v16.0.1 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest May 4, 2018
@tombuildsstuff tombuildsstuff requested a review from a team May 4, 2018 07:46
@tombuildsstuff
Copy link
Contributor Author

Tests look good for this PR 👍

@tombuildsstuff
Copy link
Contributor Author

@metacpp there's often breaking changes between versions of the Azure SDK - as such we upgrade to the new builds often (we should probably be going to each minor release as we do in the other repositories); in this case this PR upgrades to v16.1 since we're currently 4 releases behind (actually, v16.2 was released shortly after I opened this PR); in this PR specifically I'm looking to upgrade to the new SDK version to try and fix an issue with Key Vault.

Unfortunately since all of the repositories in the terraform-providers org move quickly there's never going to be a good time to do an SDK upgrade since there's always going to be conflicts with open PR's. In general we upgrade any PR's which are close enough to merge to the currently used version of the SDK - and ask users to upgrade the PR's to the new SDK version (with instructions for how to do this via Govendor) otherwise.

Thanks!

@tombuildsstuff tombuildsstuff changed the title Upgrading to v16.0.1 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest Upgrading to v16.1.0 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest May 4, 2018
@metacpp metacpp changed the title Upgrading to v16.1.0 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest Upgrading to v16.2.1 of Azure/azure-sdk-for-go / v10.8.1 of Azure/go-autorest May 4, 2018
@metacpp
Copy link
Contributor

metacpp commented May 4, 2018

@tombuildsstuff we can plan the SDK upgrade as the sprint work item if the strategy here is always to stay with latest version of Go SDK. Then we can have dedicated resource to work on it, everyone is aware of this at the beginning, none will be surprised. This is to avoid repeated testing effort for each PR contributor.

I have a hard requirement to upgrade to v16.2.1 and I made a commit to your PR. I will kick off another round of full tests.

@tombuildsstuff
Copy link
Contributor Author

@metacpp I'm going to revert your commit and push another to fix it - since every file has changed.

Regarding planning SDK upgrades, given SDK releases happen so frequently (multiple times a week in some cases) that that'd be an unnecessary admin overhead - especially where these shouldn't conflict with other PR's in most situations (as is the case in our other providers, such as AWS). In addition - SDK changes are only handled by HashiCorp at this time - since we need to be acutely aware of what's changed - such that we'll handle specific PR's as outlined above.

@metacpp
Copy link
Contributor

metacpp commented May 6, 2018

@tombuildsstuff The local commit only has very few files, hmm, weird. Anyway, thanks for the revision.

Copy link
Member

@mbfrahry mbfrahry left a comment

Choose a reason for hiding this comment

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

LGTM!

@tombuildsstuff
Copy link
Contributor Author

Tests look fine 👍

@tombuildsstuff tombuildsstuff merged commit c2ddf4f into master May 7, 2018
@tombuildsstuff tombuildsstuff deleted the azure-sdkv16 branch May 7, 2018 14:27
tombuildsstuff added a commit that referenced this pull request May 7, 2018
@ghost
Copy link

ghost commented Mar 31, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants