JetStream enum PascalCase serializer

[mtmk]

System.Text.Json serializer has some difficult settings serializing enums. Before we just relied on unconventional property names in code but proper way is to use PascalCase. Unfortunately the JSON options also did not allow snake_case conversion also I didn't want to force System.Text.Json v8 onto net6.0 targets (if that supports it). Hence we now have a JetStream specific JSON enum convertor.

[oising]

I'm a bit confused -- system.text.json 8.0.0 has a net6 target and supports snake/kebab case naming policies. Why reinvent the wheel? The net8 work on performance is second to none; I'm worried we may be leaving some perf on the table with a home rolled solution, so to speak.

edit: Okay, you don't want to force consumers of the library onto v8. It' early morning here, lol. There's always multitargeting/conditional msbuild package include and #if :P

[mtmk]

I'm a bit confused -- system.text.json 8.0.0 has a net6 target and supports snake/kebab case naming policies. Why reinvent the wheel? The net8 work on performance is second to none; I'm worried we may be leaving some perf on the table with a home rolled solution, so to speak.

I agree we don't want to maintain something that is already out there. Performance shouldn't be a big concern here as much since stream and consumer configs won't be on any hot path in vast majority of the cases.

edit: Okay, you don't want to force consumers of the library onto v8. It' early morning here, lol. There's always multitargeting/conditional msbuild package include and #if :P

:o) That's true. I didn't want to complicate it unnecessarily. We can revisit once we drop net6.0. Would that be acceptable?

edit: @oising see the last commit

[caleblloyd]

Look great, enums feel much more natural now. It is a bit annoying System.Text.Json doesn't have this until 8.0. I don't particularly like the idea of forcing an upgrade of System.Text.Json to 8.0 if on net6.0 then, so I like this approach.

LGTM

[caleblloyd]

Also doesn't really seem worth it to maintain different models using precompiler directives (#if) - the main gain of Snake Case naming policy is readability I believe, and #if is the least readable 😄

[mtmk]

Also doesn't really seem worth it to maintain different models using precompiler directives (#if) - the main gain of Snake Case naming policy is readability I believe, and #if is the least readable 😄

oops sorry. just saw your comment. Have a look at the last commit. It didn't look too bad. Happy to revert though 😄

[mtmk]

The only word didn't quite match the convention was Workqueue (as opposed to WorkQueue) but it might be Rider's English dictionary, looks like it is a word!

[caleblloyd]

Actually yea that is not so bad, it's mainly limited to just 1 file, and 1-liners in other files. Will just need to make sure the test coverage hits both net6.0 and net8.0 for all of those codepaths now - but I think you already adjusted the test matrix for that?

[mtmk]

Actually yea that is not so bad, it's mainly limited to just 1 file, and 1-liners in other files. Will just need to make sure the test coverage hits both net6.0 and net8.0 for all of those codepaths now - but I think you already adjusted the test matrix for that?

yes. it's xunit magic. when the project is multi-targeted xunit is running the tests for all targets.

[oising]

Nice work!