Skip to content

Fix: Add missing proguard rules#1297

Merged
marandaneto merged 1 commit into
mainfrom
fix/proguard-rules
Mar 1, 2021
Merged

Fix: Add missing proguard rules#1297
marandaneto merged 1 commit into
mainfrom
fix/proguard-rules

Conversation

@marandaneto

@marandaneto marandaneto commented Feb 26, 2021

Copy link
Copy Markdown
Contributor

📜 Description

Fix: Add missing proguard rules

💡 Motivation and Context

rules are per module, so some of them need to be placed in all of our android modules.
missing 2 important rules that keep our stack traces unambiguous

💚 How did you test it?

📝 Checklist

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

🔮 Next steps

@marandaneto marandaneto requested a review from a team February 26, 2021 12:50
@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #1297 (bb56ab4) into main (4620134) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1297      +/-   ##
============================================
+ Coverage     75.64%   75.65%   +0.01%     
- Complexity     1785     1786       +1     
============================================
  Files           183      183              
  Lines          6207     6207              
  Branches        622      622              
============================================
+ Hits           4695     4696       +1     
  Misses         1232     1232              
+ Partials        280      279       -1     
Impacted Files Coverage Δ Complexity Δ
...jul/src/main/java/io/sentry/jul/SentryHandler.java 73.10% <0.00%> (ø) 31.00% <0.00%> (ø%)
...rc/main/java/io/sentry/logback/SentryAppender.java 89.74% <0.00%> (+1.28%) 23.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4620134...bb56ab4. Read the comment docs.

@bruno-garcia

Copy link
Copy Markdown
Member

Seeing @maciejwalkowiak tagged here made me realize you probably want to tune the CODEOWNERS so the Android part is owned by manoel and the java part by maciej

@bruno-garcia bruno-garcia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

RSLGTM

Comment thread sentry-android-timber/proguard-rules.pro
@marandaneto

Copy link
Copy Markdown
Contributor Author

Seeing @maciejwalkowiak tagged here made me realize you probably want to tune the CODEOWNERS so the Android part is owned by manoel and the java part by maciej

not sure, whos gonna review our own PRs then? :)

@marandaneto marandaneto merged commit 4886071 into main Mar 1, 2021
@marandaneto marandaneto deleted the fix/proguard-rules branch March 1, 2021 08:11
@bruno-garcia

Copy link
Copy Markdown
Member

Seeing @maciejwalkowiak tagged here made me realize you probably want to tune the CODEOWNERS so the Android part is owned by manoel and the java part by maciej

not sure, whos gonna review our own PRs then? :)

You can assign ppl for review when needed. If you need me or Philipp to review tag us.
It's just too painful to get tagged on literally all PRs automatically

@bruno-garcia

Copy link
Copy Markdown
Member

Also don't think it's good use of Maciejs time to get tagged in all android PRs. He doesn't care when we bump ndk.

@marandaneto

Copy link
Copy Markdown
Contributor Author

Seeing @maciejwalkowiak tagged here made me realize you probably want to tune the CODEOWNERS so the Android part is owned by manoel and the java part by maciej

not sure, whos gonna review our own PRs then? :)

You can assign ppl for review when needed. If you need me or Philipp to review tag us.
It's just too painful to get tagged on literally all PRs automatically

are we talking about the mobile team or @maciejwalkowiak being tagged on an Android PR?

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