Skip to content

Conversation

@ueman
Copy link
Collaborator

@ueman ueman commented Jun 20, 2021

📜 Description

Adds support for user feedback.
Builds on #505 and thus should be merged after it.

💡 Motivation and Context

Closes #215

💚 How did you test it?

📝 Checklist

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

🔮 Next steps

@ueman

This comment has been minimized.

@ueman ueman changed the base branch from main to feat/attachments June 20, 2021 18:12
@codecov-commenter
Copy link

codecov-commenter commented Jun 20, 2021

Codecov Report

❌ Patch coverage is 71.15385% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.32%. Comparing base (828de72) to head (cb459f4).
⚠️ Report is 1801 commits behind head on main.

Files with missing lines Patch % Lines
dart/lib/src/sentry_envelope_item.dart 50.00% 4 Missing ⚠️
dart/lib/src/sentry_user_feedback.dart 80.00% 4 Missing ⚠️
dart/lib/src/hub_adapter.dart 33.33% 2 Missing ⚠️
dart/lib/src/noop_hub.dart 0.00% 2 Missing ⚠️
dart/lib/src/sentry.dart 0.00% 2 Missing ⚠️
dart/lib/src/hub.dart 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
- Coverage   91.77%   91.32%   -0.45%     
==========================================
  Files          73       74       +1     
  Lines        2371     2422      +51     
==========================================
+ Hits         2176     2212      +36     
- Misses        195      210      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marandaneto
Copy link
Contributor

What tests do we want to have for the UI, if the UI is wanted at all? Do we want to have widget and golden image tests?

we actually don't need a UI at all, but its nice that you have added to the sample, so we can keep it.

spec is here https://develop.sentry.dev/sdk/features/#user-feedback
we can tackle #441 later as its not a blocker and it'd require changes on native SDKs too probably.

@marandaneto
Copy link
Contributor

this PR branches from feat/attachments, I'd not get blocked by this though because user feedback and attachments are different features that could be used together, but don't depend on each other.

overall nice PR, unit tests and so on are missing but I like where this is going :) thanks @ueman

@philipphofmann philipphofmann self-requested a review June 22, 2021 12:23
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

API looks good. Didn't check the dart details, though.

@ueman ueman force-pushed the feat/user-feedback branch from cba13f2 to f6d2ad9 Compare June 28, 2021 06:38
@ueman ueman force-pushed the feat/attachments branch from bbb21c5 to c8a936c Compare June 28, 2021 11:14
@ueman ueman force-pushed the feat/user-feedback branch from f6d2ad9 to 0988ed0 Compare June 28, 2021 11:18
@ueman ueman force-pushed the feat/attachments branch from 5e256c9 to 1cc42ef Compare June 29, 2021 08:37
@marandaneto marandaneto added this to the 6.0.0 milestone Jun 29, 2021
@ueman ueman force-pushed the feat/user-feedback branch from 0988ed0 to 14f8787 Compare July 2, 2021 18:25
@ueman ueman force-pushed the feat/attachments branch from ae0dc9f to cd6aae6 Compare July 2, 2021 18:25
@ueman ueman force-pushed the feat/user-feedback branch from 14f8787 to b604c06 Compare July 2, 2021 18:35
@ueman ueman force-pushed the feat/user-feedback branch from b604c06 to 2b85f76 Compare July 5, 2021 06:18
@ueman
Copy link
Collaborator Author

ueman commented Jul 5, 2021

The UI is in the example app for now. I do intend to clean it up and write tests for it in a follow-up PR.

@ueman ueman marked this pull request as ready for review July 5, 2021 08:48
@marandaneto
Copy link
Contributor

@ueman is it ready for final code review? @bruno-garcia or @philipphofmann will do it, thanks

@ueman
Copy link
Collaborator Author

ueman commented Jul 5, 2021

@ueman is it ready for final code review? @bruno-garcia or @philipphofmann will do it, thanks

Yep, it's ready now.

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

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

Base automatically changed from feat/attachments to main July 13, 2021 12:55
@ueman ueman merged commit 994ddd4 into main Jul 13, 2021
@ueman ueman deleted the feat/user-feedback branch July 13, 2021 13:34
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.

Support for user feedback

6 participants