Support merge keys #32
Conversation
| } | ||
| } | ||
|
|
||
| private[this] val flattener: FlatteningConstructor = new FlatteningConstructor |
There was a problem hiding this comment.
Are you sure this is threadsafe? SnakeYAML's docs explicitly absolve them of thread safety. It could be that this particular bit of SnakeYAML is threadsafe, but maybe it's best to create a new FlatteningConstructor in the call to parse and pass it through all the yamlToJsons just to be sure? It sure seems like it ought to be safe since you're only using it for flatten, but Constructor does seem to have all kinds of non-threadsafe mutable state, and it wouldn't be trivial to ensure it's impossible for it to get touched by this.
There was a problem hiding this comment.
Yeah, better to be safe than sorry I guess. I'll make this change later.
Codecov Report
@@ Coverage Diff @@
## master #32 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 4 4
Lines 102 106 +4
Branches 6 5 -1
=====================================
+ Hits 102 106 +4
Continue to review full report at Codecov.
|
As discussed in #31. I'm like 95% sure this use of
SafeConstructoris thread-safe, but I'm not very familiar with SnakeYAML so if someone wants to check my work I'd appreciate it.I can add more specific tests if anyone wants, but this seemed reasonable and keeps us at 100% coverage.