Skip to content

Vendor JSON Writer/Reader: Refactor unknown Interface#1633

Merged
denrase merged 4 commits into
feat/vendor-json-writer-readerfrom
feat/vendor-json-writer-reader-interfaces
Aug 26, 2021
Merged

Vendor JSON Writer/Reader: Refactor unknown Interface#1633
denrase merged 4 commits into
feat/vendor-json-writer-readerfrom
feat/vendor-json-writer-reader-interfaces

Conversation

@denrase

@denrase denrase commented Aug 5, 2021 •
edited
Loading

Copy link
Copy Markdown
Collaborator

#skip-changelog

📜 Description

  • Move the unknown getter/setter requirement to own interface.

💡 Motivation and Context

  • While implementing serialization/deserialization, the need occured to also support types which do not keep around unknown objects. One example would be SentryId or enums.
  • With this approach, we can use JsonSerializable and JsonDeserializer similar to adapters.

💚 How did you test it?

  • Add unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

@denrase denrase self-assigned this Aug 5, 2021
@denrase denrase changed the base branch from main to feat/vendor-json-writer-reader August 5, 2021 12:31
@codecov-commenter

codecov-commenter commented Aug 5, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1633 (39139ab) into feat/vendor-json-writer-reader (b65016f) will decrease coverage by 0.12%.
The diff coverage is 100.00%.

❗ Current head 39139ab differs from pull request most recent head ecc145e. Consider uploading reports for the commit ecc145e to get more accurate results
Impacted file tree graph

@@                         Coverage Diff                          @@
##             feat/vendor-json-writer-reader    #1633      +/-   ##
====================================================================
- Coverage                             77.40%   77.27%   -0.13%     
- Complexity                             2375     2392      +17     
====================================================================
  Files                                   211      215       +4     
  Lines                                  8010     8107      +97     
  Branches                                912      921       +9     
====================================================================
+ Hits                                   6200     6265      +65     
- Misses                                 1409     1442      +33     
+ Partials                                401      400       -1
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/UserFeedback.java 97.26% <100.00%> (+4.10%) ⬆️
sentry/src/main/java/io/sentry/Hub.java 73.00% <0.00%> (ø)
sentry/src/main/java/io/sentry/IHub.java 90.47% <0.00%> (ø)
sentry/src/main/java/io/sentry/Sentry.java 41.49% <0.00%> (ø)
sentry/src/main/java/io/sentry/NoOpHub.java 47.50% <0.00%> (ø)
sentry/src/main/java/io/sentry/HubAdapter.java 5.08% <0.00%> (ø)
...main/java/io/sentry/protocol/MeasurementValue.java 0.00% <0.00%> (ø)
...try/spring/boot/SentrySpanWebClientCustomizer.java 100.00% <0.00%> (ø)
...ain/java/io/sentry/openfeign/SentryCapability.java 71.42% <0.00%> (ø)
...ring/tracing/SentrySpanClientWebRequestFilter.java 0.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b65016f...ecc145e. Read the comment docs.

@denrase denrase marked this pull request as ready for review August 5, 2021 13:18
@denrase denrase merged commit 4bd87fe into feat/vendor-json-writer-reader Aug 26, 2021
@denrase denrase deleted the feat/vendor-json-writer-reader-interfaces branch August 26, 2021 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bruno-garcia bruno-garcia Awaiting requested review from bruno-garcia

@maciejwalkowiak maciejwalkowiak Awaiting requested review from maciejwalkowiak

1 more reviewer

@marandaneto marandaneto marandaneto approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@denrase denrase

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@denrase @codecov-commenter @marandaneto