Skip to content

Add name and geo to User#2556

Merged
denrase merged 14 commits intomainfrom
feat/user-name-and-geo
Mar 28, 2023
Merged

Add name and geo to User#2556
denrase merged 14 commits intomainfrom
feat/user-name-and-geo

Conversation

@denrase
Copy link
Collaborator

@denrase denrase commented Feb 21, 2023

📜 Description

  • Add name property to User
  • Add Geo class
  • Add geo property to User

💡 Motivation and Context

Relates to getsentry/sentry-dart#1065

💚 How did you test it?

Added unit tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Implement on cocoa and use in hybrid SDKs

@github-actions
Copy link
Contributor

github-actions bot commented Feb 21, 2023

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against a40841e

@denrase denrase marked this pull request as ready for review February 21, 2023 10:29
@denrase denrase requested review from philipphofmann and removed request for adinauer, markushi, romtsn and stefanosiano February 21, 2023 10:29
@github-actions
Copy link
Contributor

github-actions bot commented Feb 21, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 322.90 ms 359.18 ms 36.28 ms
Size 1.73 MiB 2.34 MiB 626.62 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
d81684e 235.73 ms 328.76 ms 93.03 ms

App size

Revision Plain With Sentry Diff
d81684e 1.73 MiB 2.26 MiB 547.78 KiB

Previous results on branch: feat/user-name-and-geo

Startup times

Revision Plain With Sentry Diff
96e2af6 412.22 ms 432.49 ms 20.26 ms
87bd976 341.90 ms 446.66 ms 104.76 ms
061e3dd 329.84 ms 393.36 ms 63.52 ms
2686d20 362.39 ms 393.20 ms 30.81 ms
8b36629 344.80 ms 381.90 ms 37.10 ms

App size

Revision Plain With Sentry Diff
96e2af6 1.73 MiB 2.34 MiB 626.19 KiB
87bd976 1.73 MiB 2.34 MiB 626.56 KiB
061e3dd 1.73 MiB 2.34 MiB 626.62 KiB
2686d20 1.73 MiB 2.34 MiB 626.73 KiB
8b36629 1.73 MiB 2.34 MiB 626.76 KiB

@philipphofmann
Copy link
Member

@denrase, did you tag me for review for a specific reason? If not, Roman or Markus could also have a look at this PR.

Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

Looking great, thanks for contributing - please have a look at my comments 🚀

@denrase denrase requested a review from markushi February 27, 2023 09:59
Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

Nice one, LGTM!

@codecov
Copy link

codecov bot commented Mar 7, 2023

Codecov Report

Patch coverage: 83.33% and no project coverage change.

Comparison is base (70c2097) 81.32% compared to head (a40841e) 81.33%.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2556   +/-   ##
=========================================
  Coverage     81.32%   81.33%           
- Complexity     4192     4208   +16     
=========================================
  Files           336      337    +1     
  Lines         15489    15561   +72     
  Branches       2021     2031   +10     
=========================================
+ Hits          12597    12657   +60     
- Misses         2099     2110   +11     
- Partials        793      794    +1     
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/protocol/Geo.java 81.48% <81.48%> (ø)
sentry/src/main/java/io/sentry/protocol/User.java 89.52% <88.23%> (-0.25%) ⬇️
sentry/src/main/java/io/sentry/JsonSerializer.java 98.03% <100.00%> (+0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

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

@denrase wanna go ahead and merge it?

@denrase
Copy link
Collaborator Author

denrase commented Mar 28, 2023

@romtsn A UI running on CI test seems to be flaky. AFAIK this should not be connected to this PR. Can we merge or is there something to do first?

@romtsn
Copy link
Member

romtsn commented Mar 28, 2023

@romtsn A UI running on CI test seems to be flaky. AFAIK this should not be connected to this PR. Can we merge or is there something to do first?

merge it!

@denrase denrase merged commit d390819 into main Mar 28, 2023
@denrase denrase deleted the feat/user-name-and-geo branch March 28, 2023 12:17
@denrase
Copy link
Collaborator Author

denrase commented Mar 28, 2023

Thx @markushi & @romtsn for the reviews!

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.

4 participants