JSON Export Reapply #45
Conversation
No longer serialize into strings, instead use Vec<u8> as it is what we're writing to the files. Fixup comments
| @@ -51,16 +54,19 @@ impl ExportEntry { | |||
|
|
|||
| /// The ResultExportType enum is used to denote the desired form | |||
| /// of exporter to use for a given file. | |||
| #[derive(Clone)] | |||
| pub enum ResultExportType { | |||
There was a problem hiding this comment.
Maybe consider using this enum just to track the type of export and pass the file path as a separate argument? Just a thought.
There was a problem hiding this comment.
Well, as I was looking at it, these two pieces of data are intrinsically linked to one-another. I didn't really see any benefit in breaking them apart, just to have to bring them back together or store the file name somewhere else. However, I just noticed that there is no reason to derive clone on this type and it can safely be removed.
| csv_file: Option<&'a str>, | ||
| } | ||
|
|
||
| fn create_exporter(targets: ExportTargetList) -> Option<ExportManager> { |
There was a problem hiding this comment.
I have an idea of how to refactor this slightly, but I can do that in a separate PR. I will add you as a reviewer.
There was a problem hiding this comment.
Yeah, this was next on my list, after I brought the markdown branch into PR scope
|
Great stuff! Thank you very much (again). |
Replaces #43, and relates to #42, fixup the commits and clean up some comments