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(dynamite): json schema reference resolving #1953

Merged
merged 1 commit into from
Apr 24, 2024

Conversation

Leptopoda
Copy link
Member

I did not find a better solution yet.
In theory this already allows us to fetch schemas fro other sources like local filesystem or remote servers.
I did not observe any performance penalty.

Towards: #933

@Leptopoda Leptopoda requested a review from provokateurin April 22, 2024 17:48
Copy link

codecov bot commented Apr 22, 2024

Codecov Report

Attention: Patch coverage is 70.37037% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 29.31%. Comparing base (42885c6) to head (a3c55c1).
Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1953      +/-   ##
==========================================
+ Coverage   28.97%   29.31%   +0.34%     
==========================================
  Files         245      245              
  Lines       76496    76519      +23     
==========================================
+ Hits        22161    22430     +269     
+ Misses      54335    54089     -246     
Flag Coverage Δ *Carryforward flag
dynamite 22.46% <70.37%> (+10.41%) ⬆️
dynamite_end_to_end_test 62.98% <ø> (ø)
dynamite_runtime 82.65% <ø> (ø) Carriedforward from 42885c6
neon_dashboard 92.56% <ø> (ø)
neon_framework 38.36% <ø> (ø)
neon_talk 97.01% <ø> (ø)
nextcloud 26.52% <ø> (ø)
sort_box 90.90% <ø> (ø) Carriedforward from 42885c6

*This pull request uses carry forward flags. Click here to find out more.

Files Coverage Δ
...namite/dynamite/lib/src/models/openapi/schema.dart 65.85% <81.81%> (+15.85%) ⬆️
...mite/dynamite/lib/src/models/openapi/schema.g.dart 57.61% <62.50%> (+31.60%) ⬆️

... and 9 files with indirect coverage changes

Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Could this have some tests? I see that it is not breaking anything because none of the generated code has changed, but that is no guarantee it won't break in the future.

@Leptopoda Leptopoda force-pushed the fix/dynamite/reference_resolving branch from 6903677 to 6cb2a05 Compare April 24, 2024 08:06
@Leptopoda
Copy link
Member Author

We already had the e2e tests for this but I added some directly dynamite nevertheless

@Leptopoda Leptopoda requested a review from provokateurin April 24, 2024 08:07
@provokateurin
Copy link
Member

Commit message has a typo and should be fix(dynamite): json schema reference resolving

@Leptopoda Leptopoda force-pushed the fix/dynamite/reference_resolving branch from 6cb2a05 to a3c55c1 Compare April 24, 2024 08:39
@Leptopoda Leptopoda changed the title fix(dynamite): json schema referenec resolving fix(dynamite): json schema reference resolving Apr 24, 2024
@Leptopoda Leptopoda merged commit 23728ea into main Apr 24, 2024
10 checks passed
@Leptopoda Leptopoda deleted the fix/dynamite/reference_resolving branch April 24, 2024 09:00
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