-
Notifications
You must be signed in to change notification settings - Fork 281
Disabling some "unused" warnings #610
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
Conversation
I'd submitted code that's never actually used in the android/snippets apps. Ideally we'd include it in an app, but in the meantime, disabling the "unused" warnings. Also removed one line (outside of a published-on-DAC code block) that turned out not to be needed. No changes to the code that's included on DAC, and the code still compiles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if you can use UNUSED_PARAMETER instead of UNUSED in one place. I also think we should use all capital letters.
misc/src/main/java/com/example/snippets/PreloadManagerKotlinSnippets.kt
Outdated
Show resolved
Hide resolved
misc/src/main/java/com/example/snippets/PreloadManagerKotlinSnippets.kt
Outdated
Show resolved
Hide resolved
|
|
||
| import androidx.annotation.Nullable; | ||
|
|
||
| @SuppressWarnings("Unused") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Java all caps:
@SuppressWarnings("UNUSED")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just wanted to confirm here that it's @SuppressWarnings() in Java, but @Suppress() in Kotlin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually -- when I amend my commit, I still get the warning "Method 'doSomethingAndRelease()' is never used" for this Java file (but not for the Kotlin file).
Is there something I'm doing wrong with @SuppressWarnings()? Right now I have:
@SuppressWarnings("UNUSED")
// [START android_backgroundwork_wakelock_release_java]
void doSomethingAndRelease() throws MyException {
try {
wakeLock.acquire();
doTheWork();
} finally {
wakeLock.release();
}
}
// [END android_backgroundwork_wakelock_release_java]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks reasonable to me. Can you clarify where you are seeing the morning? Is this in Android Studio or a when you run a command line lint command? I can try your steps to reproduce.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in Android Studio, when I try to push the commit to GitHub--it pops up a screen saying "there are these warnings, are you sure you want to push?"
But when I try compiling the "misc" module, I get no warnings. So if the answer is "pay attention to that, don't worry about the warning you get when you push", I'm happy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we need to use @SuppressWarnings("unused") in Java. Android Studio lint only respects this if the "unused" is lowercase.
I'm thinking we should use lower case for both Kotlin: @Suppress("unused") and Java @SuppressWarnings("unused"). The capitalized version only work in Kotlin, so I would prefer to be consistent across languages by using lowercase.
misc/src/main/java/com/example/snippets/backgroundwork/WakeLockSnippetsKotlin.kt
Outdated
Show resolved
Hide resolved
I'd submitted code that's never actually used in the android/snippets apps. Ideally we'd include it in an app, but in the meantime, disabling the "unused" warnings. Also removed one line (outside of a published-on-DAC code block) that turned out not to be needed. No changes to the code that's included on DAC, and the code still compiles. (I also see some areas where the actual code snippet is generating warnings, but I'll resolve those in separate PRs that can be reviewed by the appropriate SMEs for those docs.)
I'd submitted code that's never actually used in the android/snippets apps. Ideally we'd include it in an app, but in the meantime, disabling the "unused" warnings. Also removed one line (outside of a published-on-DAC code block) that turned out not to be needed. No changes to the code that's included on DAC, and the code still compiles. (I also see some areas where the actual code snippet is generating warnings, but I'll resolve those in separate PRs that can be reviewed by the appropriate SMEs for those docs.)
…-code-cleanup # Conflicts: # misc/src/main/java/com/example/snippets/PreloadManagerKotlinSnippets.kt # misc/src/main/java/com/example/snippets/backgroundwork/WakeLockSnippetsJava.java # misc/src/main/java/com/example/snippets/backgroundwork/WakeLockSnippetsKotlin.kt
|
Cartland -- I made the updates you suggested, but I still seem to get a warning in |
Just to be clear, are you asking me to change all the |
Per our chat, I'll change all the suppress parameters to lower-case and re-push, stay tuned! |
I was getting some warnings in Java code that should have been suppressed. It looks like this is because Java expects the @SuppressWarnings parameters to be in lowercase. Kotlin doesn't seem to care about case, but I'm changing those ones too to be consistent.
|
It looks like changing to "unused" fixed that warning! (I have one other unrelated warning, but it's about code that's on DAC so I'll want to do it in a separate CL where I can get the SME's approval.) Let me know if you see any more issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* main: (74 commits) Add basic WindowInsetsRulers cases (#621) Adding shadows code snippets (#620) Update the wear preview code Update state based (#619) add WindowInsetsRulers snippet (#616) Migrate snippets from dac/training/wearables/tiles/screen-size (#617) Resolve warnings in the Wakelock code snippets (#615) Disabling some "unused" warnings (#610) Add android_wear_tile_version_fallback (#614) Add viewmodel screen for KMP (#613) Fix missing snippet (#612) Add KMP ViewModel snippets (#611) docs(modifiers): Add TODOs for inspectableProperties lint warnings (#589) Fix: Prevent memory leak from implicit SAM conversion (#588) Fix(gestures): Handle pointer events inside awaitPointerEventScope (#590) Lint: Use specialized state holders to avoid autoboxing (#591) Refactor: Rename Composable functions to follow naming conventions (#592) Refactor(text): Use KTX extension for isDigitsOnly check (#593) Refactor(SearchBar): Reorder modifier parameter to follow convention (#594) Add padding to avoid overwriting TimeText (#529) ...

I'd submitted code that's never actually used in the android/snippets apps. Ideally we'd include it in an app, but in the meantime, disabling the "unused" warnings.
Also removed one line (outside of a published-on-DAC code block) that turned out not to be needed.
No changes to the code that's included on DAC, and the code still compiles.