Hey Gorkem, There is a bunch of housework to do around merging the PR. Thank you for providing two implementations, but we have to be sure that all other platforms at least have issues filed for following support.
I am currently attending a W3C meeting and don't have much time this week, but I have this flagged in my inbox to follow up with. I will slate it for next week. If anyone else wants to take on merging and filing outstanding issues for platform parity, that would be nice too :) On 4/8/13 9:51 PM, "Gorkem Ercan" <[email protected]> wrote: >Can we possibly proceed with this PR? Is there anything that I need to do >on my part? >-- >Gorkem > > >On Sat, Apr 6, 2013 at 1:56 AM, Gorkem Ercan <[email protected]> >wrote: > >> Great! PRs are updated with the suggested changes >> -- >> Gorkem >> >> >> On Fri, Apr 5, 2013 at 11:36 PM, Andrew Grieve >><[email protected]>wrote: >> >>> Yep, sounds good to me! >>> >>> >>> On Fri, Apr 5, 2013 at 3:28 PM, Gorkem Ercan <[email protected]> >>> wrote: >>> >>> > I see your point... I have unified the template config.xmls on all >>> > platforms, perhaps it would be a better idea to differentiate the >>> template >>> > config.xml per platform so that the values from other platforms are >>>not >>> > spread to all... CLIs template can still carry the values of all >>> platforms >>> > that is its purpose anyway. If that sounds fine I will update the PR. >>> > -- >>> > Gorkem >>> > >>> > >>> > On Fri, Apr 5, 2013 at 12:04 AM, Andrew Grieve <[email protected]> >>> > wrote: >>> > >>> > > On Thu, Apr 4, 2013 at 4:48 PM, Gorkem Ercan >>><[email protected]> >>> > > wrote: >>> > > >>> > > > On Thu, Apr 4, 2013 at 9:17 PM, Andrew Grieve >>><[email protected] >>> > >>> > > > wrote: >>> > > > >>> > > > > Some feedback: >>> > > > > >>> > > > > + <access origin=".*"/> >>> > > > > + <access origin="http://127.0.0.1*"/> <!-- allow local >>>pages >>> > -->` >>> > > > > >>> > > > > Why have the second line? it's made redundant by the first, and >>> when >>> > we >>> > > > do >>> > > > > serve files locally, we do so via file: URLs. >>> > > > > >>> > > > > >>> > > > I have copied this use from the android's existing config.xml >>>[1]. >>> It >>> > > does >>> > > > not make much sense to me either but since I did not have time to >>> > > > investigate it deeply and did not want to break anything, I kept >>>it >>> as >>> > it >>> > > > is. We can easily remove it if you think it will not harm >>>anything >>> for >>> > > > Android. >>> > > > >>> > > > [1] >>> > > > >>> > > > >>> > > >>> > >>> >>>https://github.com/apache/cordova-android/blob/master/framework/res/xml/ >>>config.xml >>> > > >>> > > >>> > > ah, okay. Yeah, I think it should just be removed. >>> > > >>> > > >>> > > >>> > > > >>> > > > >>> > > > >>> > > > >>> > > > > >>> > > > > >>> > > > > + <log level="DEBUG"/> >>> > > > > >>> > > > > I don't see <log> in the widget spec. Seems like a good idea >>>to be >>> > able >>> > > > to >>> > > > > se a log level, but probably would be more appropriate as a >>> > > <preference>, >>> > > > > and out of the scope of this change. >>> > > > > >>> > > > > >>> > > > > >>> > > > Correct, this is not on the spec but Android at the moment is >>> actually >>> > > > using log element to configure its logging level and therefore >>>it is >>> > > > present on the current android config.xml. I also agree that >>> logging is >>> > > not >>> > > > in the scope of this change and it is probably a fair amount of >>> work. >>> > If >>> > > > you think the effect on Android is trivial, I can remove it from >>>the >>> > > > templates. >>> > > > >>> > > > >>> > > Cool, didn't know that. I think for now, let's at least keep this >>> > confined >>> > > to Android's config.xml then or convert it to a <preference> >>>before we >>> > > expose it to other platforms. >>> > > >>> > > >>> > > > >>> > > > > >>> > > > > Using <param> for the class name does more align with the spec, >>> but >>> > > using >>> > > > > $platform-package to address platform differences seems out of >>> place >>> > > > > compared with how these are addressed by CLI's config.xml >>>(using >>> > > > > gap:platform attributes or <platform> tags). What would be >>>better >>> (I >>> > > > think) >>> > > > > would be to have <param name="package" value="foo"/>, and use >>> > > CLI/plugman >>> > > > > to deal with putting in the correct versions per platform. >>> > > > > >>> > > > > E.g., if iOS adds something to its template config.xml file, I >>> don't >>> > > want >>> > > > > to have to add this value to each platform's config.xml file. >>> > > > > >>> > > > > >>> > > > > >>> > > > I wanted to keep it within the spec because any extensions to the >>> > widget >>> > > > spec needs a wider concencus . I used the $platform-package >>> convention >>> > > but >>> > > > could be any of the other options as long as it allows us to mark >>> the >>> > > > element with a platform. I think this is something that needs to >>>be >>> > > agreed >>> > > > and continued. >>> > > > >>> > > > >>> > > I'm not so much focused on what the attribute is called, but >>>rather I >>> > feel >>> > > more strongly that we shouldn't put iOS-specific things in >>>Android's >>> > > config.xml and vice-versa. Reason being is that if you extrapolate >>> this >>> > to >>> > > our ~10 platforms, that's a tonne of duplicated settings, and any >>> tweak >>> > you >>> > > want to make to one, you need to then make in ~10 repositories. >>> > > >>> > > Instead, what we should do is move to using cordova-cli's >>>config.xml >>> be >>> > the >>> > > unified version: >>> > > >>> > > >>> > >>> >>>https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;a=blob;f=templ >>>ates/www/config.xml;h=206bc5698780acda1863c4612cc36291a15b0472;hb=HEAD >>> > > >>> > > Note though that we don't have <feature> in here, since they are >>> defined >>> > in >>> > > plugin.xml. e.g.: >>> > > >>> > > >>> > >>> >>>https://github.com/MobileChromeApps/chrome-cordova/blob/master/plugins/s >>>ocket/plugin.xml >>> > > >>> > > >>> > > >>> > > >>> > > >>> > > >>> > > > >>> > > > >>> > > > > >>> > > > > >>> > > > > >>> > > > > >>> > > > > On Thu, Apr 4, 2013 at 1:11 PM, Shazron <[email protected]> >>> wrote: >>> > > > > >>> > > > > > Yes please. I want to get this pain all over with :) >>> > > > > > >>> > > > > > >>> > > > > > On Thu, Apr 4, 2013 at 9:56 AM, Filip Maj <[email protected]> >>> wrote: >>> > > > > > >>> > > > > > > Sounds good! >>> > > > > > > >>> > > > > > > Ping Shaz, Andrew, Michal, Joe, Simon, and anyone else >>> involved >>> > in >>> > > > > > Android >>> > > > > > > & iOS. >>> > > > > > > >>> > > > > > > On 4/4/13 5:08 AM, "Gorkem Ercan" <[email protected]> >>> > wrote: >>> > > > > > > >>> > > > > > > >Hi Filip, >>> > > > > > > >Thanks for looking at this. I have just updated the PR(s) >>> with >>> > > > > corrected >>> > > > > > > >config.xml ids for templates on all projects. >>> > > > > > > > >>> > > > > > > >I am also planning to send a PR for updates to doc once >>>these >>> > are >>> > > > > > > >integrated. >>> > > > > > > >-- >>> > > > > > > >Gorkem >>> > > > > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > > > > >On Wed, Apr 3, 2013 at 6:29 PM, Filip Maj <[email protected]> >>> > wrote: >>> > > > > > > > >>> > > > > > > >> Hey Gorkem, >>> > > > > > > >> >>> > > > > > > >> Thanks for this and putting the effort into kick >>>starting >>> > this. >>> > > > > Sorry >>> > > > > > > >> about the late reply. >>> > > > > > > >> >>> > > > > > > >> I like the changes (made a minor comment re: widget >>> element id >>> > > in >>> > > > > the >>> > > > > > > >> github pull request). Correctly adopting the spec should >>> help. >>> > > > > > > >>Leveraging >>> > > > > > > >> several <param> elements inside a <feature> element, one >>> param >>> > > per >>> > > > > > > >> platform, to describe the resolution of native plugin >>> source >>> > > from >>> > > > > > > >> cordova.exec service label [1] is elegant. >>> > > > > > > >> >>> > > > > > > >> I'm up for +1'ing these changes. Should help with our >>> ongoing >>> > > > plugin >>> > > > > > > >> tooling work too! >>> > > > > > > >> >>> > > > > > > >> Unless other people have problems with this approach, >>>I'll >>> aim >>> > > to >>> > > > > > merge >>> > > > > > > >> this stuff in on Friday. Perhaps some of the core >>> maintainers >>> > > for >>> > > > > > > >>Android >>> > > > > > > >> and iOS can review those particular changes (I trust >>>your >>> > > > judgement >>> > > > > > more >>> > > > > > > >> than my high-level understanding :P). If that all checks >>> out, >>> > we >>> > > > can >>> > > > > > set >>> > > > > > > >> up issues for the Windows Phone platforms, BlackBerry, >>>and >>> > other >>> > > > > > > >>platforms. >>> > > > > > > >> >>> > > > > > > >> [1] >>> > > https://github.com/apache/cordova-android/pull/41/files#L2R57 >>> > > > > > > >> >>> > > > > > > >> On 4/1/13 2:24 PM, "Anis KADRI" <[email protected]> >>> wrote: >>> > > > > > > >> >>> > > > > > > >> >I would like this to be reviewed/merged as well because >>> > > > config.xml >>> > > > > > > >> >differences are becoming a pain in terms of plugin >>> > management. >>> > > > > > > >> > >>> > > > > > > >> >Android has a /cordova/plugins. iOS had a >>> /cordova/plugins in >>> > > 2.4 >>> > > > > and >>> > > > > > > >>now >>> > > > > > > >> >has a /widget/plugins. BlackBerry 10 has a >>> /widget/plugins >>> > and >>> > > > is >>> > > > > > > >> >following the spec by using "feature" instead of >>>"plugin". >>> > > > > > > >> > >>> > > > > > > >> >Whatever we decide I would like to have some kind of >>> > uniformity >>> > > > > > across >>> > > > > > > >> >platforms. >>> > > > > > > >> > >>> > > > > > > >> >-a >>> > > > > > > >> > >>> > > > > > > >> > >>> > > > > > > >> >On Thu, Mar 28, 2013 at 12:05 PM, Gorkem Ercan >>> > > > > > > >> ><[email protected]>wrote: >>> > > > > > > >> > >>> > > > > > > >> >> Hi All, >>> > > > > > > >> >> I am working on a set of plugins for Eclipse that >>>will >>> > > > eventually >>> > > > > > be >>> > > > > > > >> >>part >>> > > > > > > >> >> of the JBoss IDE. I seem to have similar >>>requirements to >>> > > > > > cordova-cli >>> > > > > > > >>and >>> > > > > > > >> >> noticed that some of the things that are planned for >>> cli is >>> > > > well >>> > > > > > > >>aligned >>> > > > > > > >> >> with our plans. So I am hoping to contribute as much >>>as >>> I >>> > > can. >>> > > > > > > >> >> >>> > > > > > > >> >> We also use W3 widget packaging spec based config.xml >>> as a >>> > > > > blanket >>> > > > > > to >>> > > > > > > >> >> configure a Cordova App for all platforms. However >>>there >>> > are >>> > > > > > > >> >>differences on >>> > > > > > > >> >> the config.xml that each platform consumes compared >>>to >>> the >>> > W3 >>> > > > > spec >>> > > > > > > >>and >>> > > > > > > >> >>as >>> > > > > > > >> >> you can imagine a more uniform platform behaviour >>>makes >>> our >>> > > > life >>> > > > > a >>> > > > > > > >>bit >>> > > > > > > >> >> better. So I have tried to take a shot at unifying >>>the >>> > > > > differences >>> > > > > > > >> >>between >>> > > > > > > >> >> platforms and created pull requests for android[1]. >>> iOS[2] >>> > > and >>> > > > > > > >>CLI[3]. I >>> > > > > > > >> >> think with these PRs JIRA[4] for migrating from >>> <plugin> to >>> > > > > > <feature> >>> > > > > > > >> >> should be resolved (at least for iOS and Android) and >>> its >>> > > > > parent[5] >>> > > > > > > >> >>should >>> > > > > > > >> >> be updated. >>> > > > > > > >> >> >>> > > > > > > >> >> The changes are compatible with the existing >>> config.xmls on >>> > > iOS >>> > > > > and >>> > > > > > > >> >> Android, I think it will be even possible to mix and >>> match >>> > > the >>> > > > > new >>> > > > > > > >> >>syntax >>> > > > > > > >> >> <feature> with the old ones. >>> > > > > > > >> >> >>> > > > > > > >> >> [1] https://github.com/apache/cordova-android/pull/41 >>> > > > > > > >> >> [2] https://github.com/apache/cordova-ios/pull/45 >>> > > > > > > >> >> [3] https://github.com/apache/cordova-cli/pull/7 >>> > > > > > > >> >> [4] https://issues.apache.org/jira/browse/CB-1109 >>> > > > > > > >> >> [5] https://issues.apache.org/jira/browse/CB-1108 >>> > > > > > > >> >> >>> > > > > > > >> >> -- >>> > > > > > > >> >> Gorkem >>> > > > > > > >> >> >>> > > > > > > >> >>> > > > > > > >> >>> > > > > > > > >>> > > > > > > > >>> > > > > > > >-- >>> > > > > > > >-- >>> > > > > > > >Gorkem >>> > > > > > > >http://www.gorkem-ercan.com >>> > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > > >>> > > > >>> > > > -- >>> > > > -- >>> > > > Gorkem >>> > > > http://www.gorkem-ercan.com >>> > > > >>> > > >>> > >>> > >>> > >>> > -- >>> > -- >>> > Gorkem >>> > http://www.gorkem-ercan.com >>> > >>> >> >> >> >> -- >> -- >> Gorkem >> http://www.gorkem-ercan.com >> > > > >-- >-- >Gorkem >http://www.gorkem-ercan.com
