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

fix: update dependencies #229

Merged
merged 7 commits into from May 8, 2020
Merged

fix: update dependencies #229

merged 7 commits into from May 8, 2020

Conversation

ldez
Copy link
Contributor

@ldez ldez commented May 7, 2020

This PR is mainly because of the github.com/Azure/go-autorest dependency is not compatible recent version of github.com/Azure/go-autorest.
The changes made by github.com/Azure/go-autorest about go modules produce is compatibility issue.

I also updated some others dependencies.

@ldez ldez requested a review from a team as a code owner May 7, 2020 01:35
@netlify
Copy link

netlify bot commented May 7, 2020

Deploy preview for open-api ready!

Built with commit a094c96

https://deploy-preview-229--open-api.netlify.app

@keiko713
Copy link
Contributor

keiko713 commented May 8, 2020

Thanks for the contribution, @ldez !
Do you have any reference regarding the incompatibility? I'm not familiar with it so any pointer would be appreciated.

@ldez
Copy link
Contributor Author

ldez commented May 8, 2020

The module github.com/Azure/go-autorest was divided in several sub-modules:

  • github.com/Azure/go-autorest/autorest
  • github.com/Azure/go-autorest/autorest/adal
  • github.com/Azure/go-autorest/autorest/azure/cli
  • ...

https://github.com/Azure/go-autorest#using-with-go-modules

what's happen when you try to use open-api in another project that use a more recent version of autorest:

github.com/go-acme/lego/v3/providers/dns/azure imports
        github.com/Azure/go-autorest/autorest: ambiguous import: found package github.com/Azure/go-autorest/autorest in multiple modules:
        github.com/Azure/go-autorest v12.0.0+incompatible (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest@v12.0.0+incompatible/autorest)
        github.com/Azure/go-autorest/autorest v0.5.0 (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest/autorest@v0.5.0)
github.com/go-acme/lego/v3/providers/dns/azure imports
        github.com/Azure/go-autorest/autorest/azure/auth: ambiguous import: found package github.com/Azure/go-autorest/autorest/azure/auth in multiple modules:
        github.com/Azure/go-autorest v12.0.0+incompatible (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest@v12.0.0+incompatible/autorest/azure/auth)
        github.com/Azure/go-autorest/autorest/azure/auth v0.1.0 (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest/autorest/azure/auth@v0.1.0)
github.com/go-acme/lego/v3/providers/dns/azure imports
        github.com/Azure/go-autorest/autorest/to: ambiguous import: found package github.com/Azure/go-autorest/autorest/to in multiple modules:
        github.com/Azure/go-autorest v12.0.0+incompatible (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest@v12.0.0+incompatible/autorest/to)
        github.com/Azure/go-autorest/autorest/to v0.2.0 (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest/autorest/to@v0.2.0)
github.com/go-acme/lego/v3/providers/dns/azure imports
        github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2017-09-01/dns imports
        github.com/Azure/go-autorest/autorest/azure: ambiguous import: found package github.com/Azure/go-autorest/autorest/azure in multiple modules:
        github.com/Azure/go-autorest v12.0.0+incompatible (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest@v12.0.0+incompatible/autorest/azure)
        github.com/Azure/go-autorest/autorest v0.5.0 (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest/autorest@v0.5.0/azure)
github.com/go-acme/lego/v3/providers/dns/azure imports
        github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2017-09-01/dns imports
        github.com/Azure/go-autorest/autorest/validation: ambiguous import: found package github.com/Azure/go-autorest/autorest/validation in multiple modules:
        github.com/Azure/go-autorest v12.0.0+incompatible (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest@v12.0.0+incompatible/autorest/validation)
        github.com/Azure/go-autorest/autorest/validation v0.1.0 (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest/autorest/validation@v0.1.0)
github.com/go-acme/lego/v3/providers/dns/azure imports
        github.com/Azure/azure-sdk-for-go/services/dns/mgmt/2017-09-01/dns imports
        github.com/Azure/go-autorest/tracing: ambiguous import: found package github.com/Azure/go-autorest/tracing in multiple modules:
        github.com/Azure/go-autorest v12.0.0+incompatible (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest@v12.0.0+incompatible/tracing)
        github.com/Azure/go-autorest/tracing v0.1.0 (/home/ldez/sources/go/pkg/mod/github.com/!azure/go-autorest/tracing@v0.1.0)

@keiko713
Copy link
Contributor

keiko713 commented May 8, 2020

ohh makes sense, thank you for the explanation! +1 for updating that part.
I can see that you updated several other dependencies. I'm generally +1 for updating patch version things, but I can see that one package (cenkalti/backoff) is having 2 major version bump, which is concerning to me.
Do you know if that one is not introducing any breaking changes with major version updates? One thing I can see is that v4 requires minimum go 1.12 (which should be okay with this project), but it would be great if you could check that too.

@ldez
Copy link
Contributor Author

ldez commented May 8, 2020

The v3 of backoff is just the v2 but with the right module name: cenkalti/backoff@v2.2.1...v3.0.0

But the v3 contains a bug: go-acme/lego#1043

The v4 works well in Lego and Traefik without any regression.

Copy link
Contributor

@keiko713 keiko713 left a comment

Choose a reason for hiding this comment

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

sounds great, thank you so much for all the explanation!

@keiko713 keiko713 merged commit e72e2eb into netlify:master May 8, 2020
@ldez ldez deleted the fix/dependencies branch May 8, 2020 17:27
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.

None yet

2 participants