CLOUDP-304967: Add IPA rule for description format #592
Conversation
|
|
||
| function checkViolationsAndReturnErrors(description, path) { | ||
| const upperCaseStart = new RegExp(`^[A-Z]`); | ||
| const periodEnd = new RegExp(`[.]$`); |
There was a problem hiding this comment.
you need to consider the old case https://github.com/mongodb/openapi/blob/3915e0a1a921c00e2ccdb30a37e8b0b8a2120f5f/tools/spectral/.spectral.yaml#L56C10-L56C27 a lot of descriptions end with a markdown table |
There was a problem hiding this comment.
Ah thanks for pointing it out. We have another guideline to not allow inline tables, so perhaps in this case if it ends with a table we should ignore rather than say it's adopted
There was a problem hiding this comment.
There's another IPA guideline stating (and there will be another validation rule for) Descriptions should not include inline tables, as this may not work well with all tools, particularly generated client code. Should we consider descriptions ending with Markdown tables valid?
There was a problem hiding this comment.
Yeah that's what I was thinking, so for this rule instead of saying . and | is valid, we can ignore cases where it ends with | and not collect adoption
There was a problem hiding this comment.
I saw your comment after updating the page, sorry :D I am okay with ignoring them.
There was a problem hiding this comment.
NP! Will update 👍
There was a problem hiding this comment.
Split them into two separate rules, and for the full stop test ignoring inline table endings
| - '$.components.parameters[*]' | ||
| then: | ||
| function: 'IPA117HasDescription' | ||
| xgen-IPA-117-description-starts-with-uppercase: |
There was a problem hiding this comment.
Q: Can't we merge them into same rule as you did before, and giving inline table marks as functionOptions
There was a problem hiding this comment.
Was thinking about it initially though I didn't want to skip validating that the first letter is uppercase because it ends with |, so I split them into two to have clearer logic in the validator. Though we could add this as a function option for the description-ends-with-period, wdyt?
There was a problem hiding this comment.
Good context. Based on your explanation current state in PR SGTM
4 participants
Proposed changes
Adds IPA rules
xgen-IPA-117-description-starts-with-uppercaseandxgen-IPA-117-description-ends-with-period.xgen-IPA-117-description-ends-with-periodwill ignore descriptions ending with|as some descriptions have inline tables.~20 violations for components and rule
xgen-IPA-117-description-ends-with-periodthat were not caught with the legacy rule. Will go and fix these after merge (exceptions shouldn't be needed).Jira ticket: CLOUDP-304967