Feat: Add support for dio
#688
Conversation
Codecov Report
@@ Coverage Diff @@
## main #688 +/- ##
==========================================
- Coverage 90.67% 90.59% -0.08%
==========================================
Files 97 103 +6
Lines 3152 3255 +103
==========================================
+ Hits 2858 2949 +91
- Misses 294 306 +12
Continue to review full report at Codecov.
|
| version: 6.3.0 | ||
| homepage: https://docs.sentry.io/platforms/dart/ | ||
| repository: https://github.com/getsentry/sentry-dart | ||
| issue_tracker: https://github.com/getsentry/sentry-dart/issues |
There was a problem hiding this comment.
The other packages don't have the issue_tracker field, so it should probably aligned?
# Conflicts: # flutter/analysis_options.yaml
| import 'package:sentry/sentry.dart'; | ||
| import 'package:sentry_dio/sentry_dio.dart'; | ||
|
|
||
| Future<void> main() async { |
There was a problem hiding this comment.
can we make a pub project like https://github.com/getsentry/sentry-dart/tree/main/dart/example?
so we can compile and run easily on CI as well
There was a problem hiding this comment.
Sure. I've also included it in the Flutter example app, maybe that's already enough?
There was a problem hiding this comment.
mm I guess because of paths-ignore, we'd not have a failed CI when changes don't integrate well?
I still kinda prefer the pub project facility, but it's a personal opinion.
There was a problem hiding this comment.
we can do that in a different PR, so we don't block it.
|
@ueman can we possibly get rid of the |
|
@ueman that's awesome, thank you so much, left a few comments but I focused on structure and design only, I'll let @brustolin or @denrase with a more detailed review. |
Unfortunately dio isn't configured as strict as this package, so I've included it in each file which interacts with dios API. Being stricter than dio can lead to runtime errors. If there are any better ways to handle this, I'm happy to hear about it. |
|
@ueman mind attaching a screenshot of the Dio request using the sample? with the automatically created spans. |
@marandaneto I'v attached it to the PR description at the top. |
|
@ueman happy to merge and release after addressing #688 (comment) |
Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
|
📜 Description
See #673
💡 Motivation and Context
See #673
Closes #673
💚 How did you test it?
New tests & CI pipeline
📝 Checklist
🔮 Next steps
Publish to pub.dev