Skip to content

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Feb 25, 2025

@Techassi Techassi self-assigned this Feb 25, 2025
Copy link

netlify bot commented Feb 25, 2025

Deploy Preview for stackable-docs ready!

Name Link
🔨 Latest commit 7d1df0b
🔍 Latest deploy log https://app.netlify.com/sites/stackable-docs/deploys/6819e61dbf441600087ee3e2
😎 Deploy Preview https://deploy-preview-712--stackable-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

LGTM in general, just some small remarks

> [!TIP]
> This approach can be used for fields which opt into optional (experimental) features but still providing the ability to customize using the field. Using `Option<T>` will push the responsibility of dealing with `None` (eg. using a default fallback value) to the operator.

**TODO:** Can we detect the addition of an optional field and thus enable not needing to bump / introduce a new version? We *can* somehow reliable detect `Option`, but it is not guaranteed to be the `Option` (from the standard library) we expect.
Copy link
Member

Choose a reason for hiding this comment

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

I fear it's very complicated to determine this and I'm fine not automating this for now (and rely on human intelligence to determine if the version should bumped).
If we determine the breakingness however, I think we should be looking at the generated crd.yaml, not the Rust code. Maybe there are already some tools for that already ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

I fear it's very complicated to determine this and I'm fine not automating this for now

It is indeed very complicated. I would even say impossible to do it reliably (right). I just left this to note down a random thought I had.

If we determine the breakingness however, I think we should be looking at the generated crd.yaml, not the Rust code. Maybe there are already some tools for that already ;)

Sure, that also makes sense. If we would down that road, I would still explore defining this in Rust code. This would also enforce specific rules around code. But as stated above, this was mostly just a random thought which will be removed from the final ADR anyway.

@sbernauer sbernauer moved this from Development: In Progress to Development: In Review in Stackable Engineering Apr 28, 2025
@sbernauer sbernauer moved this from Development: In Review to Development: In Progress in Stackable Engineering Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants