-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
docs(flutter): Use sentry flutter init in samples #11858
base: master
Are you sure you want to change the base?
docs(flutter): Use sentry flutter init in samples #11858
Conversation
@denrase is attempting to deploy a commit to the Sentry Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments, I think we can improve the code snippets in general to reduce the size of the snippets
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
@buenaflor I noticed some formatting issues and one codeblock is still using Sentry.init. Will udate and notify you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't add comment everywhere but if possible inline the callbacks to make the code snippets a bit more digestible
FutureOr<SentryEvent?> beforeSend(SentryEvent event, Hint hint) async { | ||
if (event.throwable is DatabaseException) { | ||
event = event.copyWith(fingerprint: ['database-connection-error']); | ||
} | ||
return event; | ||
} | ||
|
||
await Sentry.init( | ||
(options) { | ||
options.beforeSend = beforeSend; | ||
}, | ||
); | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's keep it concise, don't create another function but do it inline e.g
options.beforeSend = (event, hint) {
// etc..
return event;
}
FutureOr<SentryEvent?> beforeSend(SentryEvent event, Hint hint) async { | ||
return hint is MyHint ? null : event; | ||
} | ||
|
||
await SentryFlutter.init( | ||
(options) { | ||
options.beforeSend = beforeSend; | ||
}, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as the other about keeping concise
FutureOr<SentryEvent?> beforeSend(SentryEvent event, Hint hint) async { | ||
// Modify the event here: | ||
event = event.copyWith(serverName: null); // Don't send server names. | ||
return event; | ||
} | ||
|
||
await SentryFlutter.init( | ||
(options) { | ||
options.beforeSend = beforeSend; | ||
}, | ||
); | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as the other about keeping concise
import 'package:flutter/widgets.dart'; | ||
import 'package:sentry_flutter/sentry_flutter.dart'; | ||
|
||
Future<void> main() async { | ||
// Capture only 25% of events | ||
await SentryFlutter.init((options) => options.sampleRate = 0.25); | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need the import and add void main? at this point the user should've gone through the intro anyway, imo I'd prefer this and highlight the sampleRate line
await SentryFlutter.init(
(options) {
// Capture only 25% of events
options.sampleRate = 0.25;
},
);
wdyt
(options) { | ||
options.attachViewHierarchy = true; | ||
}, | ||
appRunner: () => runApp(MyApp()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appRunner: () => runApp(MyApp()), |
don't think we need it necessarily
```dart | ||
// Option 1: Retrieving SentryId from beforeSend | ||
SentryId sentryId = SentryId.empty(); | ||
|
||
await SentryFlutter.init((options) { | ||
options.beforeSend = (event, hint) async { | ||
sentryId = event.eventId; | ||
return event; | ||
}; | ||
}); | ||
|
||
// Option 2: Retrieving SentryId from the method capturing the event | ||
SentryId sentryId = Sentry.captureMessage("My message"); | ||
|
||
// Option 3: Retrieving SentryId from the beforeSend callback | ||
SentryId sentryId = Sentry.lastEventId; | ||
|
||
final userFeedback = SentryUserFeedback( | ||
eventId: sentryId, | ||
comments: 'Hello World!', | ||
email: '[email protected]', | ||
name: 'John Doe', | ||
); | ||
|
||
Sentry.captureUserFeedback(userFeedback); | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will we need this? I think we are going to update the feedback docs anyway: #11756
DESCRIBE YOUR PR
Tell us what you're changing and why. If your PR resolves an issue, please link it so it closes automatically.
Add samples that use
SentryFlutter.init
instead ofSentry.init
.Relates to getsentry/sentry-dart#2383
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
EXTRA RESOURCES