cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Maj <...@adobe.com>
Subject Re: Unifying the config.xml
Date Tue, 09 Apr 2013 09:18:47 GMT
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" <gorkem.ercan@gmail.com> 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 <gorkem.ercan@gmail.com>
>wrote:
>
>> Great! PRs are updated with the suggested changes
>> --
>> Gorkem
>>
>>
>> On Fri, Apr 5, 2013 at 11:36 PM, Andrew Grieve
>><agrieve@chromium.org>wrote:
>>
>>> Yep, sounds good to me!
>>>
>>>
>>> On Fri, Apr 5, 2013 at 3:28 PM, Gorkem Ercan <gorkem.ercan@gmail.com>
>>> 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 <agrieve@chromium.org>
>>> > wrote:
>>> >
>>> > > On Thu, Apr 4, 2013 at 4:48 PM, Gorkem Ercan
>>><gorkem.ercan@gmail.com>
>>> > > wrote:
>>> > >
>>> > > > On Thu, Apr 4, 2013 at 9:17 PM, Andrew Grieve
>>><agrieve@chromium.org
>>> >
>>> > > > 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 <shazron@gmail.com>
>>> wrote:
>>> > > > >
>>> > > > > > Yes please. I want to get this pain all over with :)
>>> > > > > >
>>> > > > > >
>>> > > > > > On Thu, Apr 4, 2013 at 9:56 AM, Filip Maj <fil@adobe.com>
>>> 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" <gorkem.ercan@gmail.com>
>>> > 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 <fil@adobe.com>
>>> > 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" <anis.kadri@gmail.com>
>>> 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
>>> > > > > > > >> ><gorkem.ercan@gmail.com>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


Mime
View raw message