Repository navigation
Conversation
|
|
|
Hi @pkwarren, I saw you are recently active in this project. I don't suppose you could take a look at this PR? I'd love to link to custom rules from buf check plugins! |
| // A URL that provides more human-readable information about the rule in question. | ||
| // | ||
| // Optional. | ||
| string documentation_link = 5; |
There was a problem hiding this comment.
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://')"
}
];
There was a problem hiding this comment.
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).
| // 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Adding the ability to add a documentation link to annotations (fixes #10, and precursor to fixing bufbuild/buf#4422).
I've also added a general-purpose qualifier field to facilitate rule-refinement in some way. I anticipate using this to capture api-linter's rules, but it would also be useful if checking against external systems that require extra context as to the reason of a failure and want to carry that through to the user.