-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
output/csv: Remove the support for snake_case options #3390
Conversation
Removed the options using the snake_case format, instead use the alternative camelCase versions.
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3390 +/- ##
==========================================
- Coverage 73.11% 73.10% -0.01%
==========================================
Files 258 258
Lines 19647 19641 -6
==========================================
- Hits 14364 14358 -6
Misses 4401 4401
Partials 882 882
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
FileName null.String `json:"fileName" envconfig:"K6_CSV_FILENAME"` | ||
SaveInterval types.NullDuration `json:"saveInterval" envconfig:"K6_CSV_SAVE_INTERVAL"` | ||
TimeFormat null.String `json:"timeFormat" envconfig:"K6_CSV_TIME_FORMAT"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use the json encoding on this anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of it, I changed it mostly for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm this is used in the config file ... Can we revert this as this is another breaking change.
I have no idea if somebody is using this, but breaking it seems bad to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically here
Line 46 in 8457c91
Collectors map[string]json.RawMessage `json:"collectors"` |
Removed the options using the snake_case format, instead use the alternative camelCase versions.
What?
Removed the options using the snake_case format (e.g.
file_name
), instead use the alternative camelCase versions (e.g.fileName
).Why?
Deprecation process.