Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
==========================================
- Coverage 86.56% 86.55% -0.01%
==========================================
Files 82 82
Lines 5358 5357 -1
==========================================
- Hits 4638 4637 -1
Misses 720 720 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zacharycmontoya
left a comment
zacharycmontoya
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your patience 🙏🏼
| } | ||
|
|
||
| for (; i < end; ++i) { | ||
| for (i = beg; i < end; ++i) { |
There was a problem hiding this comment.
Can we put i = 0 instead ? I'm almost sure that beg is 0 at this time and it makes the code a slightly more readable
There was a problem hiding this comment.
Make sens. Unfortunately, this PR has been opened for too long, and revisiting it for a nit is not a priority. However, please don’t hesitate to open a new PR if you'd like to revisit or continue the work
| continue; | ||
| } | ||
| if (key.empty()) continue; | ||
| value = std::string{trim(token.substr(separator + 1))}; |
There was a problem hiding this comment.
Could it be interesting to ignore the key with no value ?
There was a problem hiding this comment.
that's done in parse_tags l.210
There was a problem hiding this comment.
Sorry my comment was obviously wrong, to ignore the value with empty value*
4 participants
Description
This pull request addresses a bug in the tag parsing logic where space-separated values were incorrectly considered even when comma separators were present in the
DD_TAGSenvironment variable. According to the RFC:"This is a (mostly) backwards-compatible change to DD_TAGS parsing where we use comma-separated tags if there are any commas present in the DD_TAGS variable, and use space-separation otherwise."
The fix ensures that the presence of commas takes precedence, aligning the parsing behavior with the intended specification.
Motivation
The bug was identified during system tests, which rely on correct tag parsing to function as expected. This fix brings the implementation into compliance with the RFC and prevents inconsistent behavior in environments where mixed separators may be present.