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

fix(specs): don't extend $ref objects #3623

Merged
merged 5 commits into from
Sep 4, 2024
Merged

fix(specs): don't extend $ref objects #3623

merged 5 commits into from
Sep 4, 2024

Conversation

kai687
Copy link
Contributor

@kai687 kai687 commented Aug 30, 2024

🧭 What and Why

PR #3619 added eventName examples for different event objects in the Insights API, instead of using "Product clicked" for all events.

But, extending/overriding the example property isn't supported by the OpenAPI spec, and is ignored when rendering the docs. (tested locally with the redocly CLI).

This PR removes the examples. We provide full examples for each event in the right column, so an extra example property of the event name isn't really necessary anyway.

If I understand the specs correctly:

  • in OpenAPI 3.0.3, "This object cannot be extended with additional properties and any properties added SHALL be ignored."
  • in OpenAPI 3.1.0, we can override summary and description properties of the referenced object, but still not example.

OpenAPI 3.1.0
OpenAPI 3.0.3

🎟 JIRA Ticket:

Changes included:

  • Remove eventName examples

@algolia-bot
Copy link
Collaborator

algolia-bot commented Aug 30, 2024

✔️ Code generated!

Name Link
🪓 Triggered by 8b02220897761e9037abed1bebede600318935cf
🍃 Generated commit 47f76b79a3fa2174d0d1a92d7804c838ed50b104
🌲 Generated branch generated/fix/no-extend-ref
📊 Benchmark results

Benchmarks performed on the method using a mock server, the results might not reflect the real-world performance.

Language Req/s
swift 741

@kai687 kai687 marked this pull request as ready for review August 30, 2024 11:41
@kai687 kai687 requested a review from a team as a code owner August 30, 2024 11:41
Copy link

github-actions bot commented Aug 30, 2024

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

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

Should we remove the product clicked example of the eventName ref then? This should avoid confusion

@kai687
Copy link
Contributor Author

kai687 commented Sep 4, 2024

@shortcuts PR 3619 removed the example from the eventName prop, so it's not part of the spec anymore.

@shortcuts
Copy link
Member

@shortcuts PR 3619 removed the example from the eventName prop, so it's not part of the spec anymore.

Ah indeed bah nice, cc @JonathanMontane just fyi

@shortcuts shortcuts enabled auto-merge (squash) September 4, 2024 07:54
@shortcuts shortcuts merged commit 1f53716 into main Sep 4, 2024
25 checks passed
@shortcuts shortcuts deleted the fix/no-extend-ref branch September 4, 2024 07:54
algolia-bot added a commit that referenced this pull request Sep 4, 2024
Co-authored-by: Kai Welke <kai.welke@algolia.com>
@JonathanMontane
Copy link
Contributor

Even better then!

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.

4 participants