cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Grebnov (Akvelon)" <v-seg...@microsoft.com>
Subject RE: cordova launcher icon support https://github.com/apache/cordova-cli/pull/126
Date Fri, 18 Apr 2014 23:32:20 GMT
Hi, 

Let's finally get this finalized:) It seems we have the following PR that contains all our
changes and which is good to be merged. Axel kindly gave me a green light to create a new
PR instead of original #126. 

https://github.com/apache/cordova-cli/pull/166 

We also have a docs update

https://github.com/apache/cordova-docs/pull/201/ 

Thx!
Sergey
-----Original Message-----
From: Bryan Higgins [mailto:bryan@bryanhiggins.net] 
Sent: Friday, April 4, 2014 7:48 AM
To: dev@cordova.apache.org
Subject: Re: cordova launcher icon support https://github.com/apache/cordova-cli/pull/126

You need to rebase your branch so the new commits appear after apache:master/HEAD


On Fri, Apr 4, 2014 at 10:39 AM, Axel Nennker <ignisvulpis@gmail.com> wrote:

> This is probably a stupid question but: "Why do I see so many changed 
> files here https://github.com/apache/cordova-cli/pull/126/files
> like Readme.md which I do not want in this pull request".
> I did a "get fetch upstream" from the original repo to my local repo 
> and pushed that back to my githup repo. I thought that this makes the 
> current PR smaller.
>
>
> 2014-04-04 16:00 GMT+02:00 Bryan Higgins <bryan@bryanhiggins.net>:
>
> > Great, thanks Sergey!
> >
> > I commented on the pull request. It looks like this is almost ready 
> > to be merged.
> >
> > I'd urge any other platform maintainers to take a look / test this 
> > out ASAP.
> >
> >
> > On Thu, Apr 3, 2014 at 5:40 PM, Sergey Grebnov (Akvelon) < 
> > v-segreb@microsoft.com> wrote:
> >
> > > Pushed additional improvements [1] 1. BB10 support by Bryan (w/ 
> > > additional minor improvements) 2. Due to BB10 native packager 
> > > issue and for consistency reasons
> switched
> > > from cdv/gap: platform attribute to <platform> element. Updated 
> > > config sample can be found here[2].
> > >
> > > [1] https://github.com/AxelNennker/cordova-cli/pull/4
> > > [2]  https://gist.github.com/sgrebnov/9949313
> > >
> > > Thx!!
> > > Sergey
> > > -----Original Message-----
> > > From: Bryan Higgins [mailto:bryan@bryanhiggins.net]
> > > Sent: Thursday, April 3, 2014 9:29 PM
> > > To: dev@cordova.apache.org
> > > Subject: Re: cordova launcher icon support
> > > https://github.com/apache/cordova-cli/pull/126
> > >
> > > I've got a branch up for bb10 here:
> > >
> > >
> >
> https://github.com/blackberry/cordova-cli/commit/a3da36cdc31ea4d090f5c
> 63b2930160af474d3bc
> > >
> > > Right now it fails to build if icons exist for any other platform 
> > > or an icon element exists within <platform name="blackberry10">.
> > >
> > >
> > > On Thu, Apr 3, 2014 at 11:23 AM, Bryan Higgins 
> > > <bryan@bryanhiggins.net
> > > >wrote:
> > >
> > > > All I am saying is that from an end user perspective it would be 
> > > > nice if we were consistent. If there are problems with the 
> > > > current
> platform
> > > > tag, those should be filed as issues in JIRA and fixed. Or we 
> > > > can decide it was a mistake and go with platform attribute on 
> > > > preferences as well since it was really an undocumented feature 
> > > > up until this
> > point.
> > > >
> > > > I'm with Andrew in favouring the element since it is consistent 
> > > > with plugin.xml.
> > > >
> > > > As it stands now getIcons doesn't return icon elements specified 
> > > > within platform. So the platform config will end up with an icon 
> > > > element that the parser didn't know about.
> > > >
> > > >
> > > > On Thu, Apr 3, 2014 at 10:34 AM, Axel Nennker 
> > > ><ignisvulpis@gmail.com
> > > >wrote:
> > > >
> > > >> Which supports my "suspicion" that platform was introduced for 
> > > >> preference elements but implemented in a way that developers 
> > > >> can stick any child element into it. Which would not have any 
> > > >> consequences if a developer would do it because ConfigParser.js 
> > > >> does not honor the platform element.
> > > >> Platform in config.xml is only parsed while config.xml is 
> > > >> merged
> into
> > > >> platform-config.xml
> > > >>
> > > >> This: <platform name="ios"><name>ios hello 
> > > >> world</name></platform> does not work and the developer
gets 
> > > >> not feedback that this element is not used (even on ios).
> > > >>
> > > >>
> > > >> 2014-04-03 15:42 GMT+02:00 Bryan Higgins <bryan@bryanhiggins.net>:
> > > >>
> > > >> > I added this to edge when it came up on the list a few weeks
ago.
> > > >> >
> > > >> >
> > > >> >
> > > >>
> https://git-wip-us.apache.org/repos/asf?p=cordova-docs.git;a=blobdiff
> > > >> ;f=docs/en/edge/config_ref/index.md
> ;h=9c32672403f1ceffce4278aa7e1fa6a
> > > >>
> dd7065946;hp=2df993147957cfe6f626f8a08e4a455557889e4d;hb=7598207d0e43
> > > >>
> 95905c009c056947a5a7c9930b1a;hpb=b28cb8be613f637f28dbfd3c0db2bd193e7a
> > > >> bb51
> > > >> >
> > > >> >
> > > >> > On Thu, Apr 3, 2014 at 9:38 AM, Andrew Grieve 
> > > >> > <agrieve@chromium.org>
> > > >> > wrote:
> > > >> >
> > > >> > > Feel pretty strongly that we shouldn't introduce gap: prefix.
> > > >> > > cdv: is
> > > >> bad
> > > >> > > enough :(. I'm sure PG Build will accept whatever syntax
we 
> > > >> > > come up
> > > >> with.
> > > >> > >
> > > >> > > It's true that <platform> isn't documented (that I
know 
> > > >> > > of), but it's consistent with plugin.xml and I think being

> > > >> > > able to put
> any
> > > >> > > elements
> > > >> in
> > > >> > > there makes it quite flexible.
> > > >> > >
> > > >> > >
> > > >> > > On Thu, Apr 3, 2014 at 6:20 AM, Bryan Higgins 
> > > >> > > <bryan@bryanhiggins.net
> > > >> > > >wrote:
> > > >> > >
> > > >> > > > My point is only that we should be consistent. If the

> > > >> > > > platform
> > > >> element
> > > >> > is
> > > >> > > > used for preference, then why introduce an attribute

> > > >> > > > which
> does
> > > >> > > > the
> > > >> > same
> > > >> > > > thing for icon?
> > > >> > > >
> > > >> > > > Also, I've seen platform=, cdv:platform= and 
> > > >> > > > gap:platform= within
> > > >> the
> > > >> > > pull
> > > >> > > > requests.
> > > >> > > >
> > > >> > > >
> > > >> > > > On Thu, Apr 3, 2014 at 9:08 AM, Axel Nennker 
> > > >> > > > <ignisvulpis@gmail.com
> > > >> >
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > Hi Jonathan,
> > > >> > > > > considering how difficult is is to get this icon
thing 
> > > >> > > > > is I would
> > > >> > like
> > > >> > > to
> > > >> > > > > postpone splash screen discussion after this is
merged 
> > > >> > > > > or
> > > >> rejected.
> > > >> > > > > On the other hand there were discussions on this
list 
> > > >> > > > > about splash
> > > >> > > screen
> > > >> > > > > support/changes not so long ago. I did not join
those
> because
> > > >> > > > > even
> > > >> > this
> > > >> > > > > very small icon thing - that does not even introduce

> > > >> > > > > new elements/formats/whatnot that would require
us to 
> > > >> > > > > support it until
> > > >> > hell
> > > >> > > > > freezes - takes ages to get accepted.
> > > >> > > > >
> > > >> > > > > One thing regarding icon vs splash: I think that
> > > >> icon/launcher_icon
> > > >> > is
> > > >> > > > more
> > > >> > > > > an OS thing while splash is more a app thing.
> > > >> > > > >
> > > >> > > > > cheers
> > > >> > > > > Axel
> > > >> > > > >
> > > >> > > > >
> > > >> > > > >
> > > >> > > > > 2014-04-03 14:46 GMT+02:00 Jonathan Bond-Caron
<
> > > >> > > jbondc@gdesolutions.com
> > > >> > > > >:
> > > >> > > > >
> > > >> > > > > > On Thu Apr 3 05:06 AM, Axel Nennker wrote:
> > > >> > > > > > > It is a shame that CB-2606 is unresolved
this long. 
> > > >> > > > > > > We should
> > > >> > have
> > > >> > > > > > something
> > > >> > > > > > > rolled out soon.
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > > > +1, thoughts on splashscreens or other images?
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>

Mime
View raw message