Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

Adds conversions from json string to generic Results #1464

Merged

Conversation

willemolding
Copy link
Collaborator

PR summary

Adds some of the missing conversion implementations for JsonString. It is now possible to try_into a result type directly from a JsonString.

This was NOT possible before meaning bridge calls were really difficult to get right without resorting to serde_json::Value.

It needs extra implementations for all variations of result containing a String since this no longer implements Into<JsonString>

changelog

Please check one of the following, relating to the CHANGELOG-UNRELEASED.md

  • this is a code change that effects some consumer (e.g. zome developers) of holochain core so it is added to the CHANGELOG-UNRELEASED.md (linked above), with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
  • this is not a code change, or doesn't effect anyone outside holochain core development

@@ -449,4 +501,24 @@ pub mod tests {
String::from(JsonString::from(RawString::from(1))),
);
}

#[test]
fn result_from_json_string() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@philipbeadle this should be very similar to how you want to use it in P&P

@willemolding willemolding changed the title All conversions from json string to result option Adds conversions from json string to generic Results Jun 3, 2019
@pjkundert
Copy link
Contributor

Great work! This solves exactly the problem I ran into with both HoloFuel and the App Spec tests, when trying to serialize a Result<...> response for the hdk::send/receive round-trip. Looking forward to seeing this merged!

…l-conversions-from-json-string-to-result-option
@pjkundert
Copy link
Contributor

FYI, the branch feature-app-spec-query-results tests this JsonString Result<...> deserialization in app_spec

@pjkundert pjkundert self-requested a review June 3, 2019 14:22
Copy link
Contributor

@pjkundert pjkundert left a comment

Choose a reason for hiding this comment

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

I've tested this in the feature-app-spec-query-results branch, to return a Result<...> via hdk::send/receive, and it worked great!

@willemolding willemolding merged commit f30ce36 into develop Jun 4, 2019
@zippy zippy deleted the all-conversions-from-json-string-to-result-option branch October 4, 2019 18:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants