Making JsonFactory and HttpFactory Configurable #38
Conversation
| | ||
| @Override | ||
| public boolean equals(Object obj) { | ||
| if (!(obj instanceof FirebaseOptions)) { |
There was a problem hiding this comment.
Should equals / hashCode / toString need to be updated?
There was a problem hiding this comment.
It looks like these were implemented to support some persistence requirements, which are not really used in the SDK anymore. In the future I can see us adding even more options to this class, and having to implement all these methods each time we make a change is a bit of a problem. WDYT? Should we just get rid of the implementations?
FirebaseApp also implements these methods, but at least it performs all comparisons based on the name of the app.
There was a problem hiding this comment.
I will do this in a separate PR. The existing implementations in FirebaseOptions are ok for now.
There was a problem hiding this comment.
Please do this sooner than later. Leaving broken equals / hashCode code around is a good recipe for unhappiness.
| * and detailed documentation. | ||
| * | ||
| * @param transport HttpTransport used to communicate with the remote authentication server. | ||
| * @param jsonFactory JsonFactory used to parse JSON responses from the remote authentication |
There was a problem hiding this comment.
from the remote authentication .... server?
| Fixed the typo, and left a follow up question. |
3 participants
FirebaseCredentials, which acceptJsonFactoryandHttpTransportarguments.setJsonFactory()andsetHttpTransport()methods toFirebaseOptions.Related to #35