renamed parser functions #7248
renamed parser functions #7248
Conversation
Generated via commit 3278bd9 Download link for the artifact containing the test results: ↓ atime-results.zip
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7248 +/- ##
=======================================
Coverage 99.09% 99.09%
=======================================
Files 83 83
Lines 16089 16089
=======================================
Hits 15944 15944
Misses 145 145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@MichaelChirico I have 5 PRs outstanding, please review. |
| @@ -635,12 +635,12 @@ static void str_to_i32_core(const char **pch, int32_t *target, bool parse_date) | |||
| } | |||
| } | |||
|
|
|||
| static void StrtoI32(FieldParseContext *ctx) | |||
| static void parse_i32(FieldParseContext *ctx) | |||
There was a problem hiding this comment.
wouldn't the consistent naming here be parse_int32_str (parse_${DEST_TYPE}_${SOURCE_DESCRIPTION})?
| @@ -1123,7 +1123,7 @@ static void parse_empty(FieldParseContext *ctx) | |||
| } | |||
|
|
|||
| /* Parse numbers 0 | 1 as boolean and ,, as NA (fwrite's default) */ | |||
| static void parse_bool_numeric(FieldParseContext *ctx) | |||
| static void parse_bool_10(FieldParseContext *ctx) | |||
There was a problem hiding this comment.
I prefer the old name, right now it's only 1/0 but I'm not sure we would add another parser if we wanted to, say, include 00,01. numeric->bool seems the best description here.
| @@ -1189,7 +1189,7 @@ static void parse_bool_lowercase(FieldParseContext *ctx) | |||
| } | |||
|
|
|||
| /* Parse Y | y | N | n as boolean */ | |||
| static void parse_bool_yesno(FieldParseContext *ctx) | |||
| static void parse_bool_yn(FieldParseContext *ctx) | |||
There was a problem hiding this comment.
same here, "yes/no" is the most general description for what this parser could do, anchoring on only y/n in a sense exposes the implementation detail that this currently only applies to those two characters -- in principle eventually y e s | n o could be parsed here.
I renamed some of the parser functions to be more consistent with the wider codebase, as well as more descriptive.
These functions aren't called directly, so the diff is relatively small.
Also, making the parser array const may improve performance.