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

Improve circular refs handling #14

Merged
merged 3 commits into from
Feb 4, 2025
Merged

Improve circular refs handling #14

merged 3 commits into from
Feb 4, 2025

Conversation

P0lip
Copy link
Member

@P0lip P0lip commented Feb 4, 2025

Addresses https://datawire.slack.com/archives/C01PUJXR6CT/p1738607830906789

Summary

We already had code in place for rebundling in getHttpOperationsFromSpec function, so I made an effective use of that (on a side note - this code was pretty much no-op as things stood, the only thing it did was rewriting the top-level refs)
I considered using deference with circular="ignore", but the results were rather similar as far as the negotiated content is concerned (arguably the output prior to this change was slightly better for circular structures, as we kept at least a single copy and then left a hole). That said, I might still change it to that, as it's likely to be faster.

I tried not to touch json-schema-ref-parser (to avoid needing to fork it), but I'll have a look at some point.

Checklist

  • The basics
    • I tested these changes manually in my local or dev environment
  • Tests
    • Added or updated
    • N/A
  • Event Tracking
    • I added event tracking and followed the event tracking guidelines
    • N/A
  • Error Reporting
    • I reported errors and followed the error reporting guidelines
    • N/A

Screenshots

@P0lip P0lip self-assigned this Feb 4, 2025
@P0lip P0lip marked this pull request as ready for review February 4, 2025 08:45
@P0lip P0lip marked this pull request as draft February 4, 2025 08:48
@P0lip P0lip force-pushed the jrozek/dev/circular-refs branch from 51541fb to 91e2f60 Compare February 4, 2025 08:48
@P0lip P0lip marked this pull request as ready for review February 4, 2025 09:28
@P0lip P0lip requested a review from a team February 4, 2025 09:29
@P0lip P0lip merged commit 0d28cd2 into main Feb 4, 2025
6 checks passed
@P0lip P0lip deleted the jrozek/dev/circular-refs branch February 4, 2025 10:56
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.

2 participants