Thanks guys. I'll file a bug, and should have a pull request shortly. Andrew, for the record, I didn't see anything particular in Jira around public symbols maintenance, from a cursory search.
Cheers, Kevin On Wed, Feb 20, 2013 at 8:42 AM, Andrew Grieve <agri...@chromium.org> wrote: > Would definitely be better to expose that property as a protocol. That's > the whole point of having the protocol in the first place :P. > > I forget if I created a bug for it yet, but cutting down on the number of > public symbols is definitely a TODO for both iOS and Android codebases. > > > On Wed, Feb 20, 2013 at 11:17 AM, Kevin Hawkins < > kevin.hawkins.cord...@gmail.com> wrote: > > > Not beyond the twitch, no. :) The current de facto implementation works > for > > our uses. The latter (header) issue was honestly the first time I'd > > noticed, doing a code review where "#import CDVCommandDelegateImpl.h" was > > being added to my view controller's .m file, and I didn't understand why. > > > > I can file and fix it, either way. Would you all prefer the property type > > switch, or simply fixing the header imports? The only risk I see around > > the property change is the fact that it's public interface, so could > > generate warnings (errors?) at compile time, for people upgrading their > > existing projects. > > > > Cheers, > > Kevin > > > > > > > > On Wednesday, February 20, 2013, Michal Mocny wrote: > > > > > Thanks for mentioning this. Would you like to file that bug and/or > > submit > > > a pull request? > > > > > > Also, do you have some motivating reason for moving to a "more > > > protocol-based property"? I understand your twitch, but am curious if > > you > > > are trying to replace with another implementation? > > > > > > -Michal > > > > > > On Tue, Feb 19, 2013 at 7:59 PM, Kevin Hawkins < > > > kevin.hawkins.cord...@gmail.com <javascript:;>> wrote: > > > > > > > Hi all, > > > > > > > > I'm looking through the CDVViewController implementation details on > > iOS, > > > > and I'm not clear why its (public) commandDelegate property > references > > > the > > > > concrete implementation class of the CDVCommandDelegate protocol > > > > (CDVCommandDelegateImpl), as opposed to defining a more generic > > > > protocol-based property. From an object-oriented design standpoint, > > > that's > > > > something I didn't expect. Is there a reason that this is different > > than > > > > CDVPlugin's property definition? > > > > > > > > It's not a super big deal, though it sets off something twitchy in my > > > > brain. ;-) What does need to change if it stays as-is, however, is > > that > > > > CDVViewController.h should be #importing CDVCommandDelegateImpl.h, > not > > > > CDVCommandDelegate.h. The way it is currently, the responsibility of > > the > > > > former header's inclusion is being improperly passed on to the > > inheriting > > > > view controller class. And the latter header file's inclusion is > > > > superfluous. > > > > > > > > I figured I'd get thoughts before filing a bug one way or another. > > > > > > > > Thanks, > > > > Kevin > > > > > > > > > >
