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

Migrate to Go Modules #760

Merged
merged 1 commit into from
Apr 19, 2022
Merged

Conversation

eran-levy
Copy link
Contributor

fixes #551

had CLA issues with PR #553 that was created 2 years ago so created this new PR to make sure its clear

@eran-levy eran-levy mentioned this pull request Apr 9, 2022
@eran-levy
Copy link
Contributor Author

@edenhill @jeffwidman this is the new PR with all code changes for go modules migration.

Had issues with PR #553 branch which was 2 years ago - #553 (comment)

Copy link
Contributor

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

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

I haven't tested locally, but looks reasonable to me. Thanks for keeping this moving.

.DS_Store
.idea
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this related to go modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not very related just to ignore internal goland IDE and mac folders to eliminate mistakes, I can remove them if its not according to the standards, do you want me to revert this one?

kafka/README.md Show resolved Hide resolved
soaktest/go.mod Show resolved Hide resolved
@edenhill
Copy link
Contributor

This is great stuff! Let me know when it is ready to be merged.

@jeffwidman
Copy link
Contributor

@edenhill I think that's up to you? It looks ready to me unless you want the .gitignore change dropped... I don't feel strongly either way, and besides, it's your project, not mine. 😀

@eran-levy
Copy link
Contributor Author

@jeffwidman @edenhill we can consider this PR as done, otherwise you would like me to make any other changes?

Copy link
Contributor

@edenhill edenhill left a comment

Choose a reason for hiding this comment

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

Great stuff!

@edenhill edenhill merged commit 5ef2925 into confluentinc:master Apr 19, 2022
@edenhill
Copy link
Contributor

Thanks for doing this, @eran-levy !
And thanks for the review @jeffwidman !

@jeffwidman
Copy link
Contributor

Thanks @edenhill!

@eran-levy btw, I just realized that you only added a go.mod but not a go.sum file... is there a reason for that? While it's functional as-is, typically the go.sum file is committed for additional security.

@eran-levy
Copy link
Contributor Author

@jeffwidman you're correct, but the go.mod under the root folder doesnt include any lib dependencies so that why go.sum is missing. In the rest of the modules such as: examples/ there is a dependency so thats why go.sum exists.

So seems fine to me, no changes required

@jeffwidman
Copy link
Contributor

Oh that makes sense. Thanks!

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.

Migrate to go modules
3 participants