-
Notifications
You must be signed in to change notification settings - Fork 15
CLOUDP-306577: IPA-121: Datetime #648
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request implements a new Spectral rule (IPA-121) to ensure that any API field with a date-time format explicitly mentions the ISO 8601 standard in its description.
- Adds a new function (IPA121DateTimeFieldsMentionISO8601.js) to check for the ISO 8601 mention in date-time fields.
- Updates Spectral rulesets and documentation (IPA-121.yaml and README.md) accordingly.
- Introduces tests for various valid and invalid cases in IPA121DateTimeFieldsMentionISO8601.test.js.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/spectral/ipa/rulesets/functions/IPA121DateTimeFieldsMentionISO8601.js | Implements the new rule function to enforce ISO 8601 description requirement. |
| tools/spectral/ipa/rulesets/README.md | Documents the new rule with a brief description and usage details. |
| tools/spectral/ipa/rulesets/IPA-121.yaml | Defines the rule in the Spectral ruleset configuration. |
| tools/spectral/ipa/ipa-spectral.yaml | Adds the new IPA-121 ruleset to the overall IPA Spectral configuration. |
| tools/spectral/ipa/tests/IPA121DateTimeFieldsMentionISO8601.test.js | Provides test cases for validating the rule’s behavior. |
tools/spectral/ipa/rulesets/functions/IPA121DateTimeFieldsMentionISO8601.js
Outdated
Show resolved
Hide resolved
| // Not to duplicate the check for referenced schemas | ||
| if (!propertyObject) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Nit] I think you could do this by using the unresolved oas, and this check isn't needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a way to capture whether a property belongs to a referenced schema. Do you have any suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand the question. I tested locally to remove this check and by setting resolved: false in the rule definition, and the results are the same. Since the unresolved oas will not send matches for places where $ref is included, it will only validate inline schemas, params and schemas in the components.schemas section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, you suggested changing the resolved flag to false. Sure, makes sense 👍
tools/spectral/ipa/rulesets/functions/IPA121DateTimeFieldsMentionISO8601.js
Outdated
Show resolved
Hide resolved
lovisaberggren
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY!
Proposed changes
Jira ticket: CLOUDP-306577
Found 48 violations to fix
Checklist
Changes to Spectral
Further comments