Skip to content

Add '-Werror' for main build and suppress one deprecation warning #471

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

Merged
2 commits merged into from
Jun 10, 2019

Conversation

ghost
Copy link

@ghost ghost commented May 31, 2019

No description provided.

@ghost ghost requested a review from rsgowman May 31, 2019 00:29
@ghost ghost assigned rsgowman May 31, 2019
@googlebot googlebot added the cla: yes Override cla label May 31, 2019
@ghost
Copy link
Author

ghost commented May 31, 2019

@rsgowman The tests failed, but I cannot reproduce locally. If you know on top of your mind why this fails please let me know.

@rsgowman
Copy link
Member

The tests failed, but I cannot reproduce locally. If you know on top of your mind why this fails please let me know.

Try merging master. The first error I see here is:

/home/prow/go/src/github.com/firebase/firebase-android-sdk/firebase-firestore/src/main/java/com/google/firebase/firestore/remote/FirestoreCallCredentials.java:21: warning: [deprecation] CallCredentials2 in io.grpc has been deprecated
import io.grpc.CallCredentials2;
^
error: warnings found and -Werror specified

grpc was recently updated in master (by me), so it's entirely possible that grpc has deprecated some features between the old version and the current one. Ideally, I would've noticed this while doing the update, but of course didn't, since we didn't have -Werror enabled. This is exactly the sort of thing we'll be able to avoid thanks to your change. :)

As to what to do here, https://grpc.github.io/grpc-java/javadoc/io/grpc/CallCredentials2.html says Deprecated. the new interface has been promoted into CallCredentials. Implementations should switch back to "extends CallCredentials".
so I think we can just s/CallCredentials2/CallCredentials/g within FirestoreCallCredentials.java.

Aside from our test suite, it would likely be worthwhile to run a quick manual test to ensure everything's still fine. This class deals with auth tokens, so something that logs in/out would be a good candidate. FriendlyEats (https://github.com/firebase/friendlyeats-android) should do it. If you're unsure of how to go about this, I can walk you through it.

@rsgowman rsgowman assigned ghost and unassigned rsgowman May 31, 2019
@ghost ghost force-pushed the wuandy/AddWerrorForMainBuild branch from 451d84e to 60fb0d4 Compare June 5, 2019 12:10
Copy link
Author

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Fixed all warnings and also run fireeats against this PR, it seems to work fine.

@ghost ghost assigned rsgowman and unassigned ghost Jun 5, 2019
@ghost ghost merged commit 24fda58 into master Jun 10, 2019
@rlazo rlazo deleted the wuandy/AddWerrorForMainBuild branch September 18, 2019 18:02
@firebase firebase locked and limited conversation to collaborators Oct 10, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes Override cla size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants