- Notifications
You must be signed in to change notification settings - Fork491
Uh oh!
There was an error while loading.Please reload this page.
Conversation
src/FirebaseHttpClient_Esp8266.cpp Outdated
| class FirebaseHttpClientEsp8266 : public FirebaseHttpClient { | ||
| public: | ||
| FirebaseHttpClientEsp8266() {} | ||
| void setReuseConnection(bool reuse) override { | ||
| void setReuseConnection(bool reuse) override { |
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.
trailing space
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.
Done.
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 feels a little dirty to just keep the client open but the end result is a pretty clean integration so LGTM.
src/FirebaseHttpClient_Esp8266.cpp Outdated
| class ForceReuseHTTPClient : public HTTPClient { | ||
| public: | ||
| void end() { | ||
| if(connected()) { |
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.
another possibility is to call HTTPClient::end here with _canReuse set to true before it?
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.
I like that better only because it deviates less from the base so as the base changes we can merge with impunity.
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.
Done
skhaz commentedMay 23, 2017
It worked for me |
Merging: sorry all for the delay. |
Uh oh!
There was an error while loading.Please reload this page.
Fixes#230#286