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

refactor(deps): migrating from Dep to Go Module #64

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

rodrigo-brito
Copy link
Contributor

  • Migrated from Go Dep (Deprecated) to Go Module
  • Minor fixes in the code (errchecks, wrong int conversions and unnecessary code)

Copy link
Contributor

@joserenatosilva joserenatosilva left a comment

Choose a reason for hiding this comment

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

Thanks @rodrigo-brito!

Could you add these dependencies to go.mod?

go get github.com/sirupsen/logrus@v1.4.1
go get golang.org/x/crypto/acme/autocert@v0.0.0-20191011191535-87dc89f01550
go get github.com/spf13/afero@v1.2.2
go get golang.org/x/oauth2/internal@v0.0.0-20190402181905-9f3314589c9a

We had to install them in order to run make test

@rodrigo-brito
Copy link
Contributor Author

Hi @joserenatosilva! Thank you for the review!

  • Logrus and Afero packages are already on go.mod (Lines 20, 59)
  • go get for acme/autocert and oauth2/internal has no effect here (I think it is not a dependency)
    make test works fine here. Are you testing with go1.17?

I tested with a clean Docker image: docker run -ti --rm -v pwd:/app -w /app golang:1.17 make test and also works

Copy link
Contributor

@joserenatosilva joserenatosilva left a comment

Choose a reason for hiding this comment

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

It works as intended, must have been something on my local environment. 😓

@joserenatosilva joserenatosilva merged commit 21a4547 into globocom:master Oct 19, 2021
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.

2 participants