-
-
Notifications
You must be signed in to change notification settings - Fork 16
feat(stackable-versioned): Add conversion tracking #1056
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
base: main
Are you sure you want to change the base?
Conversation
46acd20
to
3ec678e
Compare
This uses correct error handling by letting the error accumulator handle the fallible function. Previously, this panicked, because the accumulator was dropped without being finished.
a72df96
to
7411abd
Compare
7411abd
to
4009271
Compare
/// Indicates that this field's type is a nested sub struct. The indicator | ||
/// is needed to let the macro know to generate conversion code with support | ||
/// for tracking across struct boundaries. | ||
pub nested: Flag, |
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.
This could be hidden behind the experimental_conversion_tracking
feature flag.
Or, as you mentioned, we could throw an error when it is used without the feature enabled (I actually like this idea better as it helps with discovering features).
stackable-operator-derive = { path = "../stackable-operator-derive" } | ||
stackable-versioned = { path = "../stackable-versioned" } |
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.
nit: Ideally we sort them alphabetical
c.value_is(&version.inner, |s| { | ||
// For now, only added fields need to be tracked. In the future, removals and | ||
// type changes also need to be tracked | ||
matches!(s, ItemStatus::Addition { .. }) |
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.
nit: Please use a match here, so that we can add a TODO for type changes. Also, the compiler will force us to think about new added actions
Part of stackabletech/issues#642
Note
This PR description will be adjusted.
Tracking changes across sub structs
This is a rough sketch of what enabling this will look like. Support for this is essential so effectively track all changes values. Fields will be targeted by JSONPath like selectors.