-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: Sort operator versions by semver version #336
Conversation
The pre-commit error can be fixed by installing the Node dependencies. @NickLarsenNZ do we want to do this now, or force merge to get this fix into the 24.11.0 release of stackablectl? |
What are the tradeoffs of doing it now or leaving it until after the release? |
I see two options:
|
I would tend to go with leaving it out to save all the extra work, as I'm unsure how critical this is right now. Happy to let it go in if someone says they really want it in the release though. |
Alright, then we leave it open for now and merge it for a 24.11.1 release of stackablectl. |
pre-commit action fixed and green ✔️ |
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.
LGTM, just some minor comments.
Co-authored-by: Techassi <git@techassi.dev>
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.
LGTM
Description
Before
After
Definition of Done Checklist
Author
Reviewer
Acceptance