OSI violates several Protobuf Style Guide rules

[globberwops]

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 suffix UNSPECIFIED

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 [email protected] on behalf of MBition GmbH, Provider Information

[jdsika]

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]

zero values do not use the suffix UNSPECIFIED

The enum structure of OSI has been adapted to the need of the application. The first value is UNKNOWN and the second is OTHER

Those two are sematically somehow in combination UNCECIFIED:

1. UKNOWN means that it is unclear what the value means, it can be false, non existant in the interface and unsupported or anything else
2. OTHER means 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 the ENUM. 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]

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

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]

zero values do not use the suffix UNSPECIFIED

The enum structure of OSI has been adapted to the need of the application. The first value is UNKNOWN and the second is OTHER

Those two are sematically somehow in combination UNCECIFIED:

1. UKNOWN means that it is unclear what the value means, it can be false, non existant in the interface and unsupported or anything else
2. OTHER means 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 the ENUM. 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.

Again, thank you for the clarification.

The reason for the rule is stated as

The zero value enum should have the suffix UNSPECIFIED, because a server or application that gets an unexpected enum value will mark the field as unset in the proto instance. The field accessor will then return the default value, which for enum fields is the first enum value.

This is basically what UNKNOWN is for in OSI, right?
I'll update the example .protolint.yml above to reflect that exception.