This repository has been archived by the owner on Oct 12, 2022. It is now read-only.
Serialize TelemetryItem before sending it to persistence. #44
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I didn't go the "deep copy on main thread" way as:
a) deep copy might create a lot of overhead (and it's generally a bad idea to do a lot of work on the main thread
b) is error prone
c) would have made it necessary to implement serializable & would have required touching our contract files
So I followed @crystk s idea and we now serialize directly after calling enqueue(...) and then have a queue of strings and persist them. Persistence is no longer in charge of serialization but just fileI/O.
Tests pass and from my POV it works, but I wasn't able to verify the bug in the first place.
I'd be great if @chrwend or @scsouthw have a look and merge