-
Notifications
You must be signed in to change notification settings - Fork 20
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
chore(api): remove CommandConfig from NetworkOptions #743
chore(api): remove CommandConfig from NetworkOptions #743
Conversation
/build_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.
Thanks for doing this @mwangggg! Could I get you do add a test case for this like this one?
cryostat-operator/api/v1beta1/cryostat_conversion_test.go
Lines 80 to 81 in 46c70ce
return append(tableEntries(), Entry("WS connections", (*test.TestResources).NewCryostatWithWsConnectionsSpecV1Beta1, | |
(*test.TestResources).NewCryostat)) |
I don't think we have any existing test function to create an CR with CommandConfig set, but the existing helper functions in the call graph here might make it simple:
cryostat-operator/internal/test/conversion.go
Lines 82 to 84 in 46c70ce
func (r *TestResources) NewCryostatWithIngressV1Beta1() *operatorv1beta1.Cryostat { | |
return r.addIngressToCryostatV1Beta1(r.NewCryostatV1Beta1()) | |
} |
79765e2
to
3e9b049
Compare
f0d0431
to
aac6cc8
Compare
/build_test |
86e0eac
to
dfc8401
Compare
/build_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.
Looks good, thanks Ming!
Welcome to Cryostat! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
git commit -S -m "YOUR_COMMIT_MESSAGE"
Fixes: #730