Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions buf/plugin/check/v1/annotation.proto
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,16 @@ message Annotation {
// this may reference the deleted FileDescriptor in against_file_descriptors, while file_location
// will not be present.
buf.plugin.descriptor.v1.FileLocation against_file_location = 4;
// A URL that provides more human-readable information about the rule in question.
//
// Optional.
string documentation_link = 5;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this belong on Rule instead of an annotation? Also if we intend to show the field to users we should add some constraints on the link format, for example:

string documentation_url = 5 [
  (buf.validate.field).ignore = IGNORE_IF_ZERO_VALUE,
  (buf.validate.field).string.uri = true,
  (buf.validate.field).string.max_len = 256,
  (buf.validate.field).cel = {
    id: "documentation_url_http_scheme"
    message: "documentation_url must use the https or http scheme"
    expression: "this.startsWith('https://') || this.startsWith('http://')"
  }
];

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some rules have "sub-rules" or specific sub-sections that are applicable and the current formulation of Rules within the Buf ecosystem is not granular enough, so this needs to be per-annotation, because the nature of the violation may need to point to a specific part of a rule or hint. For example https://example.com/rules/1234#dependencies.

Also, while my use case currently would be leveraging http for links, I'm not sure it's worth being quite so opinionated on the validation front. You could just as easily want to reference a URN or custom URL scheme other than http(s).

// An optional identifier that can help refine exactly how the rule was violated.
//
// Optional.
//
// Simple rules will probably not require this level of detail because it is obvious, but
// for more complex logic (such as checks involving external systems), it can be helpful to
// provide an identifier that narrows down why the rule failed.
string failure_code = 6;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example of how this will be used? If not I'd suggest leaving it off until we have a concrete use case and a way to surface this information to users.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use cases for this would be typically associated with invoking some other tool or encoding a specific manner of failure. Some examples:

  • A Visual Studio warning/error code: C4819
  • A cli exit code: 1 = lint error, 2 = configuration issue
  • A remote invocation against a service: 500 Internal Error

We could certainly skip this as it's not blocking me, but it seemed useful to be able to convey additional context on the nature of failures in a semi-generic way without pulling in some more sophisticated structured error reporting.

}