Skip to content

Conversation

sphterry
Copy link
Contributor

@sphterry sphterry commented Aug 6, 2025

Proposed changes

  • OperationId Validation rules are enabled with warning severity
  • Tests for these rules are enabled are a split into cases where the OperationID should have an override and cases where it should not
  • Insertion of type : module in the ipa/package.json in order to solve parsing error (ESM <-> CommonJS) with generateRulesetReadme.js

The next PR on this ticket will be in MMS, updating the OpenAPI sha to this commit. A message will be sent to MMS developers beforehand. These rules are still on warning severity and will not impact anything until they are set to error.

Jira ticket: CLOUDP-329722

@sphterry sphterry requested a review from a team as a code owner August 6, 2025 13:14
"license": "Apache-2.0",
"author": "MongoDB",
"main": "ipa-spectral.yaml",
"type": "module",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got this error when regenerating the ReadMe

(node:58114) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///Users/sophia.terry/openapi/tools/spectral/ipa/scripts/generateRulesetReadme.js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to /Users/sophia.terry/openapi/tools/spectral/ipa/package.json.
(Use `node --trace-warnings ...` to show where the warning was created)

No impact on the package configuration that I know of

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We use ES format instead of commonjs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to create separate/small PRs for the changes like those in future.

@wtrocki
Copy link
Member

wtrocki commented Aug 6, 2025

Great to see this landing 🥇

@sphterry sphterry requested a review from lovisaberggren August 6, 2025 13:29
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some test cases for:

  • Valid/invalid op ID overrides for the different methods
  • Usage of the verb override in standard and legacy custom methods

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, TY - anything missing ?

@sphterry sphterry requested a review from lovisaberggren August 6, 2025 15:33
Copy link
Collaborator

@lovisaberggren lovisaberggren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sphterry sphterry merged commit a4c60dc into main Aug 6, 2025
12 checks passed
@sphterry sphterry deleted the CLOUDP-329722 branch August 6, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants