-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Plugins Phase 1 #1469
Merged
k8s-ci-robot
merged 13 commits into
kubernetes-sigs:master
from
estroz:chore/rebase-plugins
Apr 28, 2020
Merged
Plugins Phase 1 #1469
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
1bba4fb
Plugin implementation: Phase 1
estroz 883e8c6
pkg/cli: validate cli struct's fields
estroz e1d2f11
pkg/scaffold: pass pkg/model/config.Config to scaffolds, not the
estroz d331eaa
pkg/cli: use --plugins to determine which plugin.Base's kubebuilder
estroz 478c233
pkg/mode/config: add arbitrary extra fields to Config under the
estroz e4b8d15
cli: check if project is v1 when validating default plugins
estroz 5d5e7d2
Any changes that break 'PROJECT' file backwards-compatibility require…
estroz 706019b
test/e2e: add v3 tests
estroz cd2d948
pkg/plugin: inject command name from Context so help messages contain
estroz 3596e21
pkg/cli: move --plugins to init, since phase 1 only allows one plugin
estroz 1ee71ec
*: fix validation of default values, update license years, and make
estroz a639be0
pkg: make added unit tests into ginkgo/gomega tests
estroz 8093011
pkg/model/config: use Version3Alpha variable name for project version…
estroz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
Will make easier remove the if to clean the code if/when we remove V1 impl
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.
Not sure if this was the intent, but using
!o.config.IsV2() && !o.config.IsV3()
means that we would need to explicitly assert that a future version (e.g. v4) supports multi-group. This is probably safer than assuming that all future versions will support multi-group.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.
Once #1355 is merged I will rebase to remove v1 checks.