Repository navigation
OSI violates several Protobuf Style Guide rules #711
Description
Describe the bug
OSI violates several Protobuf Style Guide rules, see Style Guide
- File Structure
imports are listed before package name
imports are not sorted
options are listed before package name and imports - Enums
zero values do not use the suffixUNSPECIFIED
Expected
Follow the Style Guide and explicitly list exceptions as it is done here:
"OSI uses singular instead of plural for repeated field names."
Provide a protolint config file, use protolint in CI.
---
lint:
rules:
remove:
- REPEATED_FIELD_NAMES_PLURALIZED
rules_option:
enum_field_names_zero_value_end_with:
suffix: UNKNOWN
indent:
style: 4
max_line_length:
max_chars: 120
tab_chars: 4
Regards,
Martin
Martin Stump martin.stump@mercedes-benz.com on behalf of MBition GmbH, Provider Information
Activity
jdsika commentedon Feb 13, 2023
Hi Martin,
at least for the pluralized repeated I think I can add to the discussion. We have talked about this several times in the past and one argument for not using a plural was that we had occasions in which we changed an optional message to a repeated message. The moment we then have to use a "s" at the end we would break compatibility which is why we thought it might be handy to just leave the singular.
Best regards
Carlo
jdsika commentedon Feb 13, 2023
The enum structure of OSI has been adapted to the need of the application. The first value is
UNKNOWNand the second isOTHERThose two are sematically somehow in combination
UNCECIFIED:UKNOWNmeans that it is unclear what the value means, it can be false, non existant in the interface and unsupported or anything elseOTHERmeans on the other hand that the reading was correct and that there is a clear unterstanding of what the value is but it is just not one of the values described in theENUM. In this case e.g. an application could be missing a color and using other to represent the value and add the value for the next version of OSI.globberwops commentedon Feb 13, 2023
Hi Carlo,
Thank you for the clarification.
Just to be clear, I am not calling for blindly following every rule in the Style Guide.
This specific exception is well documented, which is totally fine in my opinion.
globberwops commentedon Feb 13, 2023
Again, thank you for the clarification.
The reason for the rule is stated as
This is basically what
UNKNOWNis for in OSI, right?I'll update the example
.protolint.ymlabove to reflect that exception.