Cleanup refactor #233
Cleanup refactor #233
Conversation
| ReactNativeSupport.removeEspressoIdlingResources(reactNativeHostHolder); | ||
| } | ||
| } catch (JSONException e) { | ||
| Log.e(LOG_TAG, "cleanup cmd doesn't have stopRunner param"); |
There was a problem hiding this comment.
@simonracz I'm working on refactoring this switch/case clause (almost done), and couldn't help but wonder whether there was a particular reason no to fallback to a stopRunner=false behavior in case stopRunner was not provided. It's easy to do so using optBoolean("stopRunner", false) (instead of getBoolean()). I'd appreciate your input :)
There was a problem hiding this comment.
... or should it actually be a fallback to true?
There was a problem hiding this comment.
Hi @d4vidi ,
The short answer is, I don't remember. :)
In general treat my code as POC. So, feel free to refactor it and double check my decisions.
For this specific case, I think it would be safe to go with optBoolean here.
Cheers,
Simon
There was a problem hiding this comment.
Sounds about right :) so what's left now is to decided whether the default should be true or false :)
Main change that could affect iOS is that the cleanup command now sends a parameter
stopRunner. iOS can simply ignore it.PR to let Travis do its job.