search

fhircast / sandbox

FHIRcast Sandbox
GNU Affero General Public License v3.0
12 stars 3 forks source link

fix merge issue with renaming of LeaseSeconds #57

Closed wdvr closed 5 years ago

wdvr commented 5 years ago

New tests by @johns397 also renamed Lease_Seconds - there is a json parse issue where the lease seconds where not mapped and read by the hub. This commit fixes one occurrence of the rename that was not updated since PR #52.

@wmaethner this is also something to check on #56 now.

lbergnehr commented 5 years ago

Was the reason to rename lease seconds with an underscore simply serialization? As I'm sure you're aware, it's not really idiomatic C#. If we move to more custom serialization not relying simply on property names perhaps we could remove the underscore again.

johns397 commented 5 years ago

Leo,   The "Lease_Seconds" name was to get the C# property to work correctly but the underscore comes from the name in the spec, "hub.lease_seconds".          https://fhircast.hl7.org/specification/May2019Ballot/#subscribing-and-unsubscribing  Have you had a chance to look at the branch I created where I tried to add a test for updating an existing subscription?   Have you thought of how to change the sandbox implementation to use something other than ImmutableHashSet?     John

----Original Message-----From: Leo Bergnéhr notifications@github.com To: fhircast/sandbox sandbox@noreply.github.com Cc: johns397 johns397@aim.com; Mention mention@noreply.github.com Sent: Fri, May 17, 2019 2:41 am Subject: Re: [fhircast/sandbox] fix merge issue with renaming of LeaseSeconds (#57)

Was the reason to rename lease seconds with an underscore simply serialization? As I'm sure you're aware, it's not really idiomatic C#. If we move to more custom serialization not relying simply on property names perhaps we could remove the underscore again.— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.