The only concern I have is the deprecation path needs to be long and noisy---this is probably the biggest possible breaking change we could introduce to the platform.
Maybe even longer than our usual 6months / but wait until 3.0 Thoughts on that? On Tue, Sep 25, 2012 at 4:56 PM, Andrew Grieve <agri...@chromium.org> wrote: > Michal - Yep, good summary, that's exactly the case. > Simon - totally agree. I'll change what I've got to add a second executeV2 > which takes in a JSONArray, and have the String-based one just call that. > > The reason to need an executeV2 is threading, so I'll focus on that. > > My biggest gripe against the current signature, is that it defaults to > running things on a background thread. I expect most calls will be fast > enough to execute inline, some calls to need to run on the UI thread, and > then only some to require doing a lot of work on a background thread. > Furthermore, those that do require a background would often benefit from > doing some param/state checking on the calling thread before moving to the > background thread. > > I wouldn't be proposing a new signature if there was a way to change > isSync() from defaulting to false to defaulting to true, but I don't think > that's a safe thing to change. > > On iOS, plugins execute on the calling thread and it's up to them to > dispatch background threads if they need them. > > Michal pointed out that you can't comment on a diff in github, so I opened > a pull request with the patch to enable commenting: > https://github.com/agrieve/incubator-cordova-android/commit/a73dffc99847b14031c1138611bb8772dc9d7b7e > > > > > > On Tue, Sep 25, 2012 at 10:51 AM, Simon MacDonald <simon.macdon...@gmail.com >> wrote: > >> Here is what I was thinking on: >> >> https://issues.apache.org/jira/browse/CB-1530 >> >> In the PluginManager change the code so that is calls: >> >> plugin.execute(string, string, string); >> >> Then in the Plugin class add a new default method that does the following: >> >> public PluginResult execute(String action, String args, String callbackId) >> { >> return execute(action, new JSONArrary(args), callbackId); >> } >> >> so that all the current plugins continue to work without needing any >> changes. If someone wants to provide their own JSON parsing they can >> override the plugin.execute(string, string, string) method and do it >> themselves. >> >> Simon Mac Donald >> http://hi.im/simonmacdonald >> >> >> On Tue, Sep 25, 2012 at 10:33 AM, Michal Mocny <mmo...@chromium.org> >> wrote: >> > >> > Summarizing what I think I'm hearing: >> > >> > The current exec signature will currently: >> > (a) automatically parse JSON arguments, and >> > (b) automatically move async calls onto a background thread. >> > >> > While both of the features simplify plugin developers in most cases, >> > sometimes manual control is desired (ie, for the two bugs you link to). >> > >> > That sounds reasonable, however, I think I'm also hearing a proposal to >> > replace the existing execute signature (deprecating the current one). If >> > for the majority of cases we are happy with the current signature, then >> is >> > there perhaps a less intrusive solution? Or maybe we aren't happy with >> the >> > current signature, and this new signature is generally more future proof, >> > more performant, etc, giving us other reasons for changing? Also, how >> does >> > this compare with other platforms? >> > >> > -Michal >> > >> > >> > On Mon, Sep 24, 2012 at 11:50 PM, Andrew Grieve <agri...@google.com> >> wrote: >> > >> > > Means to address two bugs: >> > > >> > > https://issues.apache.org/jira/browse/CB-1530 >> > > https://issues.apache.org/jira/browse/CB-1532 >> > > >> > > I wanted to gather some opinions from those who have been around for >> > > longer. Here is the proposed change: >> > > https://github.com/agrieve/incubator-cordova-android/compare/ft3 >> > > >> > > My main motivation is for FileTransfer, I need to register the transfer >> > > synchronously so that a subsequent abort() will not have a race >> condition. >> > > I then perform the transfer in a background thread. I *could* implement >> > > this using the current signature by returning true in isSync() and then >> > > returning a NO_RESULT result, but I think the intentions are clearer >> with >> > > the new signature. >> > > >>