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

clean up replace directives in go.mod #560

Merged
merged 1 commit into from
Feb 28, 2024
Merged

clean up replace directives in go.mod #560

merged 1 commit into from
Feb 28, 2024

Conversation

tariq1890
Copy link
Contributor

@tariq1890 tariq1890 commented Feb 27, 2024

The replace directives aren't needed. This change does not alter the go.sum checksums, so the dependency tree is maintained as-is

This improves readability of the go.mod and makes it easier for others to discern the dependency tree of k8s-device-plugin

@tariq1890 tariq1890 changed the title clean up go.mod to include only necessary replace directives clean up replace directives in go.mod Feb 27, 2024
@ArangoGutierrez
Copy link
Collaborator

#529

Copy link
Collaborator

@ArangoGutierrez ArangoGutierrez left a comment

Choose a reason for hiding this comment

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

Duplicate of #529

@ArangoGutierrez ArangoGutierrez added the dependencies Issue/PR Pull about a dependency file label Feb 27, 2024
@elezar
Copy link
Member

elezar commented Feb 27, 2024

Thanks @tariq1890.

@klueska had the following question:

It would be good to understand why it was needed in the past, but if things actually compile without doing the explicit replace anymore, then that is clearly better.

in #529 (comment)

I also asked on k8s Slack: https://kubernetes.slack.com/archives/C09R23FHP/p1708089275684879

@tariq1890
Copy link
Contributor Author

I would say that the replace directives are mostly needed when we have conflicting transitive dependencies. It might have been the case previously, but not anymore.

If we seek to understand why it was needed , we would have to look at the git history and check the go mod graph output. That would give us insight on what the conflicting dependencies were

So long as we keep our dependencies up to date, we wouldn't encounter these transitive dependency conflicts.

Copy link
Member

@elezar elezar left a comment

Choose a reason for hiding this comment

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

I'm happy to merge this and revisit one we have some more experience with these.

It seems as if there is precedent to not include them -- e.g. helm -- and this would definitely improve our dependency experience.

@tariq1890 tariq1890 merged commit ae80a71 into main Feb 28, 2024
6 checks passed
@tariq1890 tariq1890 deleted the clean-go-mod branch February 28, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Issue/PR Pull about a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants