Skip to content
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

DM-35386 Fix Serialization of ConfigurableActionStructField #695

Merged
merged 1 commit into from Jul 6, 2022

Conversation

natelust
Copy link
Contributor

@natelust natelust commented Jul 5, 2022

Fields were not being properly initialized empty when serialized.
This caused state from a defaunt instatiation of a class to add
state back into a de-serialized object when it should not be
there.

@laurenam
Copy link
Contributor

laurenam commented Jul 5, 2022

Very nit-picky, but the PR title should have a : after the ticket number. Also, there are a few typos in your commit message:

  • defaunt instatiation -> default instantiation

I don't pretend to fully understand the fix (e.g. I don't know why the def __bool__ was necessary), but this has indeed fixed my problem as my Jenkins run on DM-34908 with this patch cherry picked has passed ci_hsc, so marking as reviewed 😄

@@ -355,8 +358,11 @@ def toDict(self, instance):
def save(self, outfile, instance):
actionStruct = self.__get__(instance)
fullname = _joinNamePath(instance._name, self.name)

# ensure that a struct is always empty before assigning to it
Copy link
Contributor

Choose a reason for hiding this comment

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

Another nit-pick: Comments are supposed to start capitalized.

Fields were not being properly initialized empty when serialized.
This caused state from a default instantiation of a class to add
state back into a de-serialized object when it should not be
there.
@natelust natelust merged commit cc9029d into main Jul 6, 2022
@natelust natelust deleted the tickets/DM-35386 branch July 6, 2022 17:11
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.

None yet

2 participants