fix(yaml): support collapsing of multiple anchor keys in step #391
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #391 +/- ##
==========================================
- Coverage 96.02% 95.99% -0.04%
==========================================
Files 69 69
Lines 5382 5440 +58
==========================================
+ Hits 5168 5222 +54
- Misses 138 140 +2
- Partials 76 78 +2
|
| value := tmpService.Content[i+1] | ||
|
|
||
| // check if key is an anchor reference | ||
| if strings.EqualFold(key.Value, "<<") { |
There was a problem hiding this comment.
EqualFold() seems unnecessary here (and in the other location).
There was a problem hiding this comment.
if the equalfold is necessary you could use a switch case comparison for some extra speed
switch key.Value {
case "<<":There was a problem hiding this comment.
i think it's a simple key.Value == "<<", no? there are no casing concerns.
There was a problem hiding this comment.
right, switch is faster tho (jump tables) but its negligible (with over 1 billion iterations it measures like 3% faster)
| // iterate through map contents (key, value) | ||
| for i := 0; i < len(tmpService.Content); i += 2 { | ||
| key := tmpService.Content[i] | ||
| value := tmpService.Content[i+1] |
There was a problem hiding this comment.
im assuming this cant panic? i couldnt get it to be nil while testing
There was a problem hiding this comment.
in the same spirit, the length is guaranteed to be an even number? (i think so?) :D
There was a problem hiding this comment.
Right I believe the map type check will always make this an even length
|
changing this to draft for now until we revisit |
#386 introduced a regression(?) where the following step configurations were no longer valid:
go-yamlasserts that the proper usage of multiple anchors in a single map is to list them:Given how many current users leverage the first configuration, this PR aims to support that format even with the upgrade to go-yaml v3.