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/unified api fixes and web example #137

Merged
merged 27 commits into from
Oct 27, 2020
Merged

Conversation

rxlabz
Copy link
Contributor

@rxlabz rxlabz commented Oct 26, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

  • Various fixes
  • add a Dart web example
  • add SentryClient tests

💡 Motivation and Context

verify everything working correctly after the unified refactoring

💚 How did you test it?

📝 Checklist

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

🔮 Next steps

@rxlabz rxlabz changed the title Fix/unified api fixes Fix/unified api fixes and web example Oct 26, 2020
@rxlabz rxlabz marked this pull request as ready for review October 26, 2020 22:31
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.

Very nice to get a web example! 🚀


```dart
pub get
webdev serve --release
Copy link
Member

Choose a reason for hiding this comment

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

What's webdev? should we propose installing that?

In the Flutter Web example I used python3 which should be installed on macOS and Linux OOTB: https://github.com/getsentry/sentry-dart/blob/main/flutter/example/run.sh#L54

Not true for Windows though. So maybe a comment:

Suggested change
webdev serve --release
# Serve the static files with your preferred dev static site server
webdev serve --release

Copy link
Contributor Author

@rxlabz rxlabz Oct 27, 2020

Choose a reason for hiding this comment

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

it's the Dart cli to run and build Dart web projects.
To be served from a static site server, the project needs to be build (webdev build ...) .
webdev serve is simplest/fastest way to run this example, so I just added a link to the webdev page.

Comment on lines 9 to 28
transaction: '/example/app',
level: SentryLevel.warning,
tags: const <String, String>{'project-id': '7371'},
extra: const <String, String>{'company-name': 'Dart Inc'},
fingerprint: const <String>['example-dart'],
userContext: const User(
id: '800',
username: 'first-user',
email: '[email protected]',
ipAddress: '127.0.0.1',
extras: <String, String>{'first-sign-in': '2020-01-01'}),
breadcrumbs: [
Breadcrumb(
message: 'UI Lifecycle',
timestamp: DateTime.now().toUtc(),
category: 'ui.lifecycle',
type: 'navigation',
data: {'screen': 'MainActivity', 'state': 'created'},
level: SentryLevel.info)
],
Copy link
Contributor

Choose a reason for hiding this comment

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

why are they removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved back fields which are "merged" with scope

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@rxlabz rxlabz merged commit b1761f4 into feature/unified-api Oct 27, 2020
@rxlabz rxlabz deleted the fix/unified branch October 27, 2020 12:13
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.

3 participants