Add a non-strict Style Guide group

[bufdev]

We've committed our draft Style Guide into etc/style/uber/uber.proto. This is the product of a lot of hashing out for what we want Protobuf files to look like within Uber. The rules inside this are inherently strict - we want there to be One Way To Do Things™ within Uber, and we want easy-to-follow rules that prevent users from making common mistakes. As Jisi on the Protobuf team at Google pointed out though, this style guide would require migration for most existing users.

What my hope is is that we have the new "recommended/strict Style Guide" that accomplishes our goals within Uber, which I think apply to the greater community - if we have a consistent way to do enums, a consistent naming convention for go_package, java_package, etc, everyone wins, and makes things like consistent output of stubs easier. We also have the "default Style Guide" that does the bare minimum enforcement.

For the record, here are the rules I have to ignore to get googleapis to pass lint:

lint:
  exclude_ids:
    - REQUEST_RESPONSE_TYPES_UNIQUE
    - REQUEST_RESPONSE_TYPES_IN_SAME_FILE
    - ENUM_FIELD_PREFIXES
    - ENUM_ZERO_VALUES_INVALID
    - FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX
    - FILE_OPTIONS_EQUAL_JAVA_PACKAGE_COM_PB
    - FILE_OPTIONS_REQUIRE_JAVA_PACKAGE

Action items:

• Go over the Style Guide as proposed more, and come to a common "strict" Style Guide.
• Figure out what belongs in the strict Style Guide vs the default Style Guide.

Copying Jisi's feedback from an email (hope this is ok Jisi!):

There are two parts in the style guide,

• Fromatting rules., e.g. spaces, empty lines, import orders. Those can be safely applied to the whole depot without causing any issues.
• Naming: e.g. packages, enums, etc. Those could affect the APIs.

For a global style guide, we should probably only enforce the format ones. For the naming, we can list several best-practices (we do have such a doc in google), but as protobuf is already widely used, each organization probably has its own naming style. For example, you can find Google API's style guide here. Another idea is we can have different profile for naming.

Some of the detailed feedback about the style guide:

• package: each organization probably has its own style of naming packages. I can image the migration cost for google to a converged package style would be impractical given the # of protos we have.
• enum default value: internally we recommend _UNSPECIFIED or _UNSET as 0. It's the proto3 design goal to not distinguish between unset and set-to-default. In current proto3 implementation, you won't be able to distinguish between the two -- has-bit is gone and setting a value to 0 is a no-op (values default to 0). The serialization will simply skip the field even if you set it to 0 value explicitly. This is also a API change and we should probably just give suggestions rather than enforce it.

[jzelinskie]

I don't think it's possible to unify the protobuf community at this point. What would be realistic might be auto-detection of different "profiles" (e.g. uber, google, default, strict). I say auto-detection because like I said, I don't think it's possible to get every protobuf user to adopt an additional configuration file.

[bufdev]

Hmm, any ideas on how to auto-detect? Trying to think of a simple heuristic or set of heuristics that would work. Maybe see what fails and choose?

For Uber that would be problematic, we have to enforce the stronger one, so letting people fall back based on failures defeats the point.

[bufdev]

@amckinney @AlexAPB I think what is needed for v1.0 is that we commit to the "strict" style guide, and provide a fallback "non-strict" style guide post-v1.0.

[peats-bond]

Agreed, let's stay as opinionated as possible for 1.0.

[bufdev]

Removing v1.0 label as this issue is more than just go over the Style Guide. However, going over the Style Guide is probably the last remaining item before v1.0.

[amckinney]

Now that we've removed the concept of lint groups, is this still applicable? As it stands today, Prototool comes configured with a set of default linters (that happen to conform to Uber's needs). If users disagree with the defaults, they can simply configure their linters otherwise, ie

lint:
  rules:
    no_default: true
    add:
      - ...
      - ...

It doesn't seem like we should concern ourselves with other group configurations - they can be easily configured explicitly. There are an infinite number of opinions with regard to style, so I think that it's important to draw the line here.

@peter-edge Thoughts? Should we close this task?

[bufdev]

95% of the Protobuf community will not use this for linting because we don’t have an easy way to do this without listing linters. Having more users means more valid bug reports, which means better tooling for Uber.