Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guildeline vs Zally: MUST vs SHOULD #790

Open
dp1218 opened this issue Nov 28, 2023 · 3 comments
Open

Guildeline vs Zally: MUST vs SHOULD #790

dp1218 opened this issue Nov 28, 2023 · 3 comments
Assignees

Comments

@dp1218
Copy link

dp1218 commented Nov 28, 2023

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
Copy link
Member

ePaul commented Dec 12, 2023

Discussion in the guild meeting:

  • The point of the rule is to include relevant responses, with a suggestion to group all non-specific errors under "default".
  • Zally can't really verify this, it only figures out you didn't include a default.

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:

  • If Zally identifies a clear indication that a rule is violated, this needs to be called out as is.
  • But if Zally doesn't know for sure that a rule is violated (like what we have here), it should give a hint, and just give the rule as a context.

@tkrop will give a bit more detailed explanation.

@ePaul ePaul assigned ePaul and tkrop and unassigned ePaul Jan 23, 2024
@tkrop
Copy link
Member

tkrop commented Feb 6, 2024

@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
Copy link
Member

tkrop commented Mar 6, 2024

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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants