-
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 MaxWsConnections #742
chore(api): remove MaxWsConnections #742
Conversation
74c1aad
to
df885d7
Compare
/build_test |
just realized that this may be subject to change according to #727. I can put in a dependency on it if we want to wait for that PR to be merged first. |
I think it makes sense for this to be merged into the cryostat3 branch along with the other changes Josh is currently working on. |
api/v1beta1/cryostat_types.go
Outdated
// +optional | ||
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Max WebSocket Connections",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:number"} | ||
// +kubebuilder:validation:Minimum=1 | ||
MaxWsConnections int32 `json:"maxWsConnections,omitempty"` |
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 guess maybe this should be kept in the v1beta1
spec so that it doesn't break API compatibility, but the value would actually just be ignored by the implementation and not handled on upgrades to v1beta2
.
81b028a
to
3362bb9
Compare
3362bb9
to
66b19e9
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.
Looks good, thanks @mwangggg!
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: #729