-
Notifications
You must be signed in to change notification settings - Fork 30
Re-enable vendoring #410
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
base: main
Are you sure you want to change the base?
Re-enable vendoring #410
Conversation
|
This is an example of Renovate PR opened in my fork with this PR applied: jankaluza#2 |
|
Second commit skips the codespell for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update the Makefiles to rename tidy to vendor and do the go work vendor step in the Makefiles and I guess we want the go work sync as well added to the top level Makefile. And then update the go-tidy github action to run the make vendor instead
|
@Luap99 , done in the third commit. I also renamed the tidy-in-container to vendor-in-container, but I did not find any place where this is executed - probably only manually? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @mtrmac PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, makes sense overall.
(For the record, AFAICT we need the vendor step because Renovate only does go work sync if vendoring is enabled, per #291 (comment) .)
common/Makefile
Outdated
| .PHONY: vendor | ||
| vendor: | ||
| $(GO) mod tidy | ||
| $(GO) work vendor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apples in all 3 modules: The net effect is that a top-level make vendor would run go work vendor three times. That seems unnecessary (and from a quick local check, repeated go work vendor is not faster than the first run, so this is a ~real cost, although measured in single seconds).
Maybe keep the tidy goals in the subprojects, and only turn the top-level tidy into vendor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I've kept the tidy in projects' Makefiles. The top-level Makefile has the vendor target now.
The Renovate PR to support top-level vendor directory is merged and should be deployed in the production. It should be safe to re-enable the vendoring now. Signed-off-by: Jan Kaluza <[email protected]>
We do not want to check the spelling in the code we do not own. Signed-off-by: Jan Kaluza <[email protected]>
And also run the `go work sync` in the end of the main Makefile. Signed-off-by: Jan Kaluza <[email protected]>
83aaec1 to
af0fbec
Compare
Signed-off-by: Jan Kaluza <[email protected]>
Signed-off-by: Jan Kaluza <[email protected]>
|
I also had to disable the |
The Renovate PR to support top-level vendor directory is merged and should be deployed in the production.
It should be safe to re-enable the vendoring now.