Skip to content

make ChangePoll fully owned#68

Merged
djc merged 1 commit intodjc:mainfrom
valkum:change-poll-fix
Dec 19, 2024
Merged

make ChangePoll fully owned#68
djc merged 1 commit intodjc:mainfrom
valkum:change-poll-fix

Conversation

@valkum
Copy link
Copy Markdown
Contributor

@valkum valkum commented Dec 18, 2024

There is no need for Cow as this is only deserialized and xml::deserialize uses FromXmlOwned.

This is still not working properly, as you cannot construct RequestData (which enables the deserialization of the ChangePoll struct) without RequestData being serialized with <extension> tags. See #67.


impl Transaction<ChangePoll<'_>> for Poll {}
// todo: make sure no element for <extension> is added in the request when using this.
#[derive(Debug, FromXml, ToXml)]
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we skip the ToXml derives for these types?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I also removed the FromXml from the current stub type that implements Extension.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I still see a bunch of types in the module that implement ToXml, is that necessary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No. Will take care of it tomorrow.

@valkum valkum force-pushed the change-poll-fix branch 2 times, most recently from ce23c3f to 1cb5469 Compare December 18, 2024 15:07
There is no need for Cow as this is only deserialized and xml::deserialize uses FromXmlOwned.
@djc djc merged commit ef30b6e into djc:main Dec 19, 2024
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