-
Notifications
You must be signed in to change notification settings - Fork 387
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
License format and package version updates #3919
Conversation
/test-multicluster-dataplane-e2e |
Codecov Report
@@ Coverage Diff @@
## main #3919 +/- ##
===========================================
- Coverage 64.24% 53.36% -10.89%
===========================================
Files 293 415 +122
Lines 43227 60353 +17126
===========================================
+ Hits 27772 32208 +4436
- Misses 13232 25508 +12276
- Partials 2223 2637 +414
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
In your commit message, I do not understand what you mean by "refine
related logic as others".
Also, "Let it to be updated by make codegen" should be "Let it be updated by make codegen"
@@ -82,7 +83,7 @@ else | |||
endif | |||
##@ Build | |||
|
|||
build: generate fmt vet ## Build manager binary. |
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.
is that change intentional?
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.
Yes, it's unnecessary to rerun this every time to build a binary in most cases. We should run make codegen
to update related codes if it's necessary or run make generate
If it's required to update deep copy file only.
@@ -60,5 +59,8 @@ $GOPATH/bin/informer-gen \ | |||
--output-package "${ANTREA_PKG}/multicluster/pkg/client/informers" \ | |||
--go-header-file hack/boilerplate.go.txt | |||
|
|||
go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1 |
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.
shouldn't this be added to the Docker image instead?
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.
yes, I think we can, let me refine this, thanks.
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.
Done
33d3e33
to
653dd0e
Compare
/test-multicluster-dataplane-e2e |
@luolanzone I have built and pushed |
1. Update the license format in auto-generated deepcopy file and let it be updated by `make codegen` 2. Update controller-runtime version to v0.12.1 3. Update controller-gen version to v0.9.0 4. Update codegen image to kubernetes-1.24.0-build.0 Signed-off-by: Lan Luo <luola@vmware.com>
@antoninbas I have updated corresponding files and commit/PR messages, Could you help to take a look? thanks. |
/test-multicluster-e2e |
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
/skip-all |
make codegen
Signed-off-by: Lan Luo luola@vmware.com