zalando / restful-api-guidelines

A model set of guidelines for RESTful APIs and Events, created by Zalando
https://opensource.zalando.com/restful-api-guidelines/
Creative Commons Attribution 4.0 International
2.63k stars 387 forks source link

Guildeline vs Zally: MUST vs SHOULD #790

Closed dp1218 closed 4 months ago

dp1218 commented 10 months ago

For the rule Specify Success and Error Responses, Zally returns a SHOULD:

SHOULD Specify Success and Error Responses
    operation should contain the default response
    https://zalando.github.io/restful-api-guidelines/#151
        paths > /cbos > get (lines 24-107)

But the guidelines contain a MUST:

MUST specify success and error responses [151]

Either Zally or the guideline is right. We should have the other fixed.

ePaul commented 10 months ago

Discussion in the guild meeting:

So the check says "you likely should include a default rule", but that's not the rule.

We might want to reword the rule to make it clearer, or reword the Zally check to make it clearer what is actually being checked here.

It looks like the presentation of Zally result needs to be changed to have the actual violation in the top-level, and the rule just as a context, instead of the headline.

@tfrauenstein:

@tkrop will give a bit more detailed explanation.

tkrop commented 8 months ago

@ePaul I agree, but changing the design of a rule in Zally is not that simple. We need to analyze how this observation can be best expressed with the current rule design.

tkrop commented 7 months ago

A general explanation without looking at the details of the example here:

A Zally violation is following a specific concept: it belongs to a rule and fails on a specific check that is part of the rule. Each, the rule and the check, have separate severity base on the wording of the rule title and the sentence responsible for the check. Unfortunately, our violation concept in Zally predates the advanced concept of rules and checks.

The challenge of this is, that we now represent these two aspects in a single violation message. This automatically creates inconsistencies and unexpected relations between the part of the sentence derived from the rule and the part derived from the check. To fix this issues, we need to redesign the violation concept to represent the rule and the check separately.

@dp1218 is this understandable?

dp1218 commented 4 months ago

@tkrop Yes it is. Sadly that means that we can't fully rely on Zally to know if a rule is a should or a must. Each breach has to be checked by hand. At least, I understand that those rules don't change much so this work has to be done only for the updates of the API. I think it is unschön but acceptable :)