-
Notifications
You must be signed in to change notification settings - Fork 54
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
feat(validation): Add a shared storage between all substates of a ValidateState #105
base: master
Are you sure you want to change the base?
Conversation
68b05a9
to
f0ec14e
Compare
@Arqu do you think this is something that could be merged in? |
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 did some performance testing just to be on the safe side, has no real impact unless you start hammering the struct itself.
However I think this needs some tests. The struct isn't cleared on a new substate and keys can easily collide depending on usage. Once that's out of the way, I'm happy to merge.
validation_state.go
Outdated
@@ -27,6 +27,9 @@ type ValidationState struct { | |||
Misc map[string]interface{} | |||
|
|||
Errs *[]KeyError | |||
|
|||
// ExtraData is a shared storage between all substates of a ValidationState | |||
ExtraData *map[string]interface{} |
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.
Let's rename this to AdditionalValidationData
and its respective comments/function names.
Not clearing the struct was the whole point of adding the new field. That is pretty much the difference to the Misc field. I'll create a basic test that at least confirms my usecase and make the name changes |
At the very least lets document that in the comment on the declaration. Not really sold on the use case but like that it would add a way for others to extend the implementation for their needs. Ie be more explicit that its not cleared and share the key space for its lifetime. |
db9bbef
to
9ebec95
Compare
This new field is similar to the
Misc
field from aValidationState
but is shared between all a ValidationState and all its child SubStates.In the project I'm using this library I have a few custom keywords and I need to be able to identify which are the datapaths that are associated with such keywords.
The easiest way I could think of solving this problem was by having this shared storage at the
ValidationState
level.This way, in my custom keyword
ValidateKeyword
method, I could save the necessary info and have it available as a result of the validation.