Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

transport may be set on options #203

Merged
merged 4 commits into from
Jan 3, 2020
Merged

transport may be set on options #203

merged 4 commits into from
Jan 3, 2020

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented Jan 2, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

transport may be set on options.

💡 Motivation and Context

transport should be possible to be set on options as well, unified API.

💚 How did you test it?

unit tests.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

🔮 Next steps

socket tagging because of square/okhttp#3537
but it's just a warning and an okhttp issue, not ours, so not sure if we should do this workaround only for the sake of warning free. but why? I agree with coil-kt/coil#46

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

LGTM but I'd suggest leaving it null in the options instead of creating an instance of NoOp

@codecov-io
Copy link

codecov-io commented Jan 2, 2020

Codecov Report

Merging #203 into master will increase coverage by 0.25%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #203      +/-   ##
============================================
+ Coverage     58.09%   58.35%   +0.25%     
- Complexity      552      557       +5     
============================================
  Files            72       73       +1     
  Lines          2618     2627       +9     
  Branches        229      230       +1     
============================================
+ Hits           1521     1533      +12     
+ Misses          980      978       -2     
+ Partials        117      116       -1
Impacted Files Coverage Δ Complexity Δ
...re/src/main/java/io/sentry/core/SentryOptions.java 77.68% <100%> (+0.56%) 52 <2> (+2) ⬆️
...in/java/io/sentry/core/AsyncConnectionFactory.java 100% <100%> (+18.18%) 0 <0> (-1) ⬇️
...ore/src/main/java/io/sentry/core/SentryClient.java 89.79% <100%> (+0.43%) 29 <0> (+1) ⬆️
...main/java/io/sentry/core/HttpTransportFactory.java 100% <100%> (ø) 1 <1> (?)
sentry-core/src/main/java/io/sentry/core/Dsn.java 85.29% <0%> (+2.94%) 9% <0%> (+1%) ⬆️

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 5d40505...4a38d4a. Read the comment docs.

@marandaneto marandaneto merged commit 4f15161 into master Jan 3, 2020
@marandaneto marandaneto deleted the ref/transport branch January 3, 2020 10:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants