Skip to content

SymlinkAbsolutePathStrategy enum should be wrapped in a message #56

@jmillikin-stripe

Description

@jmillikin-stripe

Protobuf's enum scoping follows traditional C/C++ rules, where the enum name is not a new level of namespacing. This means the current definition of SymlinkAbsolutePathStrategy:

message CacheCapabilities {
  enum SymlinkAbsolutePathStrategy {
    UNKNOWN = 0;
    DISALLOWED = 1;
    ALLOWED = 2;
  }
  SymlinkAbsolutePathStrategy symlink_absolute_path_strategy = 5;

will define CacheCapabilities::ALLOWED, CacheCapabilities::DISALLOWED, etc. This is very confusing because the symbols indicate that caching itself is allowed/disallowed, instead of just this one particular feature.

I recommend the following structure instead:

message SymlinkAbsolutePathStrategy {
  enum Enum {
    UNKNOWN = 0;
    DISALLOWED = 1;
    ALLOWED = 2;
  }
}

message CacheCapabilities {
  SymlinkAbsolutePathStrategy.Enum symlink_absolute_path_strategy = 5;

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions