Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Conversation

mikeland73
Copy link
Contributor

@mikeland73 mikeland73 commented Nov 9, 2022

Summary

This splits out init survey into a provider.

I think there's a few more follow ups we can do after this that will make it better, for example answers.ClusterOption == provider.CreateJetpackCluster condition can be removed (we could add a generic non error println to answers)

How was it tested?

  • linter

Is this change backwards-compatible?

yes

@mikeland73 mikeland73 requested review from ipince and LucilleH November 9, 2022 19:52
@@ -45,6 +48,11 @@ func New(opts ...padcliOption) *Padcli {
for _, opt := range opts {
opt(p)
}

if p.initSurverProvider == nil {
p.initSurverProvider = provider.DefaultInitSurveyProvider(p.clusterProvider)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a separate if statement for this, instead of adding it straight to the Padcli struct above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ugly, but it was the best way I could think of that we always use the correct cluster provider (e.g. if the user of this lib were to pass a custom cluster provider, but not a custom init provider, this ensures we assign it correctly).

If we do this above and there's a custom cluster provider then we will accidentally pass the default cluster provider.

return func(p *Padcli) {
p.persistentPreRunE = r
}
}

func WithPersistentPostRunE(r func(cmd *cobra.Command, args []string) error) padcliOption {
func WithPersistentPostRunE(r CobraFunc) padcliOption {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TY

@mikeland73 mikeland73 merged commit 3aed762 into main Nov 11, 2022
@mikeland73 mikeland73 deleted the landau/add-init-survey-provider branch November 11, 2022 01:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants