-
Notifications
You must be signed in to change notification settings - Fork 247
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
codegen: fix code-generation #1083
Conversation
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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 @fmuyassarov for the fix!
One question: which part of the auto-generation fails with the error obtaining VCS status
?
Also, maybe you could put all autogenerated changes in the last commit (removing auto-generated parts from the first commit)?
Signed-off-by: Muyassarov, Feruzjon <[email protected]>
Omit go version control information (buildvcs), otherwise go command fails to obtain vcs status as shown below: error obtaining VCS status: exit status 128 Use -buildvcs=false to disable VCS stamping. Signed-off-by: Muyassarov, Feruzjon <[email protected]>
Update generated code based on the updated from re-running make generate. Signed-off-by: Muyassarov, Feruzjon <[email protected]>
when running generate-groups.sh. node-feature-discovery/hack/generate.sh Line 20 in adea670
Yep, I myself didn't notice actually those committed (used |
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 for the update
I haven't seen the VCS status problem but it possibly occurred if you run the generation under a git worktree(?) But better to fix that problem in any case.
/assign @ArangoGutierrez
If you checkout to a58cc0a, which doesn't include |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ArangoGutierrez, fmuyassarov, marquiz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM label has been added. Git tree hash: b6671654b9291733ea60cc7d4ddeebe8dcb32d39
|
Our make-generate is currently broken as shown below and this PR fixes it by bumping mockery version and disabling buildvcs to avoid go VCS status error. More info on commit messages. Last commit is about re-running make generate (auto-generated code).
NOTE: mockery latest version is 2.22.1 but that requires mockery.yaml file to be available. I'm not sure if we really need it. But if yes, we can do it separately because this PR is about fixing the code generation.