cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gorkem Ercan <gorkem.er...@gmail.com>
Subject Re: Unifying the config.xml
Date Thu, 18 Apr 2013 06:18:40 GMT
Hey Fil,
Please let me know if I can help with anything.
--
Gorkem


On Tue, Apr 9, 2013 at 12:18 PM, Filip Maj <fil@adobe.com> wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message