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

[v9] Remove deprecated beforeScreenshot #2662

Open
wants to merge 8 commits into
base: v9
Choose a base branch
from

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Feb 5, 2025

📜 Description

Remove deprecated beforeScreenshot

💡 Motivation and Context

Closes #2642

💚 How did you test it?

Unit tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

Copy link

codecov bot commented Feb 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.44%. Comparing base (0343b5a) to head (085599f).

Additional details and impacted files
@@            Coverage Diff             @@
##               v9    #2662      +/-   ##
==========================================
+ Coverage   89.07%   92.44%   +3.36%     
==========================================
  Files         262       90     -172     
  Lines        8887     2911    -5976     
==========================================
- Hits         7916     2691    -5225     
+ Misses        971      220     -751     

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

@denrase denrase marked this pull request as ready for review February 5, 2025 13:46
Copy link
Contributor

@buenaflor buenaflor left a comment

Choose a reason for hiding this comment

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

looks good! let's do the android api bump separately

flutter/android/build.gradle Outdated Show resolved Hide resolved
@denrase denrase requested a review from buenaflor February 5, 2025 14:51
Comment on lines +22 to +24
# bump minSdkVersion to 21
gradleFile="$targetDir/perf-test-app-with-sentry/android/app/build.gradle"
sed "s/flutter.minSdkVersion/19/g" "$gradleFile" > "$gradleFile.new" && mv "$gradleFile.new" "$gradleFile"
sed "s/flutter.minSdkVersion/21/g" "$gradleFile" > "$gradleFile.new" && mv "$gradleFile.new" "$gradleFile"
Copy link
Contributor

Choose a reason for hiding this comment

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

we can revert this as well right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does not build with version 19, it seems most recent version of sentry-android does not support 19?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bildschirmfoto 2025-02-06 um 11 10 52

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yeah android bumped api from 19 to 21, let's leave it then. but we can do the minSDK bump separately

@denrase denrase requested a review from buenaflor February 6, 2025 12:39
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.

2 participants