-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
🐛 kubebuilder edit ignore the errors if the project layout is already on same type #1754
🐛 kubebuilder edit ignore the errors if the project layout is already on same type #1754
Conversation
This comment has been minimized.
This comment has been minimized.
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.
I do not think that we should do that because:
- The plugin/command kb edit can have more than one arg besides it has only the multigroup
- We can enable/disable the support via kb edit --multigroup=[true or false]
However, we could check if the project has multi-group resources scaffolded already before allow remove the support. WDYT?
The current implementation also fails if we try to edit a project which is already on the same type. Current, failure happens in the PostScaffolding of the
This change will give more clear message to the user that he is already on the same type of the project layout. |
@prafull01, I catcher the scenario 👍 However, it is a really bugfix in POV. So, wdyt about update the title with :bug; and to do the same for v2? |
65a3717
to
c65286c
Compare
Done |
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.
great 👍
/approve
@camilamacedo86 Needs /ok-to-test command to run the tests. Thanks :) |
/ok-to-test |
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.
/approve cancel
Hi @prafull01,
After tests and think about that more, I believe that it is not the best approach for we avoid this issue.
See that after this pr if we run kubebuilder edit
it will return the issue added here instead of the helper. So, WDYT about we add a check in the Dockerfile update here to ignore the issue if the projectfile==multigroup value and the error faced is can't find "COPY
?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
c65286c
to
253aec6
Compare
253aec6
to
f1fe273
Compare
f1fe273
to
d45b242
Compare
d45b242
to
40fec84
Compare
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.
/approve
Great work 👍
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, prafull01 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 |
Tested locally and the bug fixed work. /lgtm |
Description of the change
Add validation of kb edit command if the scaffolding is already on same side either
multigroup: true
ormultigroup: false
. kb edit command fails when it tries to change the Dockerfile that it doesn't find the apis or api directory respectively.Motivation of the change
Fixes: #1747