-
Notifications
You must be signed in to change notification settings - Fork 14
CLOUDP 328959: OperationID Validation functions for Create, Delete, and Update methods #805
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
tools/spectral/ipa/rulesets/functions/IPA108ValidOperationID.js
Outdated
Show resolved
Hide resolved
tools/spectral/ipa/rulesets/functions/IPA108ValidOperationID.js
Outdated
Show resolved
Hide resolved
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.
Nice work on this structure — just had a few small questions to clarify direction. Let me know what your plan about yaml inclusion, testing and exception detection is
Co-authored-by: Matt Condon <[email protected]>
@yelizhenden-mdb For clarification:
I've prioritised speed here, my motivation is to get the list of changes together ASAP. |
tools/spectral/ipa/rulesets/functions/IPA106ValidOperationID.js
Outdated
Show resolved
Hide resolved
tools/spectral/ipa/rulesets/functions/IPA106ValidOperationID.js
Outdated
Show resolved
Hide resolved
tools/spectral/ipa/rulesets/functions/IPA107ValidOperationID.js
Outdated
Show resolved
Hide resolved
tools/spectral/ipa/rulesets/functions/IPA108ValidOperationID.js
Outdated
Show resolved
Hide resolved
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.
LGTM once the lint errors are fixed
tools/spectral/ipa/rulesets/functions/IPA106ValidOperationID.js
Outdated
Show resolved
Hide resolved
tools/spectral/ipa/rulesets/functions/IPA106ValidOperationID.js
Outdated
Show resolved
Hide resolved
message: | ||
'Invalid OperationID. The Operation ID must start with the verb “create” and should be followed by a noun or compound noun. The noun(s) in the Operation ID should be the collection identifiers from the resource identifier in singular form. ', | ||
path: ['paths', '/api/atlas/v2/groups/{groupId}/invites', 'post'], | ||
severity: DiagnosticSeverity.Warning, |
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.
Just asking to learn:
I’m not fully sure how severity: off
behaves. Does it completely disable the rule, or does it still run but return errors with DiagnosticSeverity.Off
?
If it’s the first case, can we assume the rule isn't being tested at all?
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.
Severity off still runs the rule but hides the errors, which is what i want here until the package and extension are supported (ie: closer to the time of correction). The rule isn't being tested because it's not active yet, but I have run the tests on error and warning severity and they pass.
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.
Perfect! Thanks ❤️
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.
LGTM! Thanks for the changes! Exciting! 🚀
Proposed changes
Jira ticket: CLOUDP-328959
Further comments
Exception support to be added in separate ticket. Rules are not enabled.