-
Notifications
You must be signed in to change notification settings - Fork 1.6k
✨Change NewBundle
method to build bundles through working with options
#3312
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,33 @@ type bundle struct { | |
deprecateWarning string | ||
} | ||
|
||
type BundleOption func(*bundle) | ||
|
||
func WithName(name string) BundleOption { | ||
return func(opts *bundle) { | ||
opts.name = name | ||
} | ||
} | ||
|
||
func WithVersion(version Version) BundleOption { | ||
return func(opts *bundle) { | ||
opts.version = version | ||
} | ||
} | ||
|
||
func WithPlugins(plugins ...Plugin) BundleOption { | ||
return func(opts *bundle) { | ||
opts.plugins = plugins | ||
} | ||
} | ||
|
||
func WithDeprecationMessage(msg string) BundleOption { | ||
return func(opts *bundle) { | ||
opts.deprecateWarning = msg | ||
} | ||
|
||
} | ||
|
||
// NewBundle creates a new Bundle with the provided name and version, and that wraps the provided plugins. | ||
// The list of supported project versions is computed from the provided plugins. | ||
func NewBundle(name string, version Version, deprecateWarning string, plugins ...Plugin) (Bundle, error) { | ||
|
@@ -60,6 +87,41 @@ func NewBundle(name string, version Version, deprecateWarning string, plugins .. | |
}, nil | ||
} | ||
|
||
// NewBundleWithOptions creates a new Bundle with the provided BundleOptions. | ||
// The list of supported project versions is computed from the provided plugins in options. | ||
func NewBundleWithOptions(opts ...BundleOption) (Bundle, error) { | ||
bundleOpts := bundle{} | ||
|
||
for _, opts := range opts { | ||
opts(&bundleOpts) | ||
} | ||
|
||
supportedProjectVersions := CommonSupportedProjectVersions(bundleOpts.plugins...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. might be worth moving all the logic related to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that is nice for simplicity. But in Let's see what others think about this. |
||
if len(supportedProjectVersions) == 0 { | ||
return nil, fmt.Errorf("in order to bundle plugins, they must all support at least one common project version") | ||
} | ||
|
||
// Plugins may be bundles themselves, so unbundle here | ||
// NOTE(Adirio): unbundling here ensures that Bundle.Plugin always returns a flat list of Plugins instead of also | ||
// including Bundles, and therefore we don't have to use a recursive algorithm when resolving. | ||
allPlugins := make([]Plugin, 0, len(bundleOpts.plugins)) | ||
for _, plugin := range bundleOpts.plugins { | ||
if pluginBundle, isBundle := plugin.(Bundle); isBundle { | ||
allPlugins = append(allPlugins, pluginBundle.Plugins()...) | ||
} else { | ||
allPlugins = append(allPlugins, plugin) | ||
} | ||
} | ||
|
||
return bundle{ | ||
name: bundleOpts.name, | ||
version: bundleOpts.version, | ||
plugins: allPlugins, | ||
supportedProjectVersions: supportedProjectVersions, | ||
deprecateWarning: bundleOpts.deprecateWarning, | ||
}, nil | ||
} | ||
|
||
// Name implements Plugin | ||
func (b bundle) Name() string { | ||
return b.name | ||
|
Uh oh!
There was an error while loading. Please reload this page.
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.
@NikhilSharmaWe ,\
The changes in Kubebuilder CLI can still.
I mean, we can use the method here.
We need to have the old one available for those that import and use Kubebuilder as a LIB.
Also, please ensure that you rebase with master branch and it is passing in the tests for we get the PR merged.