from:"vladimir\-kotikov"

Re: wp8 support (cordova@7)

2017-04-13 Thread Vladimir Kotikov (Akvelon)

+1

-
Best regards, Vladimir

On Apr 13, 2017, at 02:10, Jesse  wrote:

+1
This goes hand in hand with removing platformApiPoly from cordova-lib.




@purplecabbage
risingj.com

On Wed, Apr 12, 2017 at 4:04 PM, Shazron  wrote:

> +1
> adieu WP8
> 
> On Wed, Apr 12, 2017 at 1:58 PM, Audrey So  wrote:
> 
>> Hi everyone! Just a quick update… we will be dropping support for windows
>> phone 8 (wp8) in cordova@7.
>> Here is the PR--> 
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-lib%2Fpull%2F539%2Fcommits=02%7C01%7Cv-vlkoti%40microsoft.com%7C5c08577ed84a4069cd9e08d481f91689%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276354237140966=ou1mFTdgAp0NIJScf%2B1O7cbkzJEOFjBvNwx5lj5IJnc%3D=0
> .
>> If you have any questions or concerns, let us know!
>> 
>> Audrey
>> 
> 


-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib issue #531: CB-12606 Fix plugin dependency installation

2017-03-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-lib/pull/531
  
ð 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-inappbrowser issue #214: CB-11248 InAppBrowser no focus on in...

2017-03-17 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-inappbrowser/pull/214
  
ð 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #228: CB-12499 UWP: Dependent external libraries speci...

2017-03-17 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/228
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: [NIGHTLY BUILDS] Missing

2017-02-15 Thread Vladimir Kotikov (Akvelon)

Hey Shazron and Fil

The thing with nightly builds is that they are running (or it’d better to say 
they had been running until Dec, 9) on Apache infrastructure. More 
specifically, this is a build job that is used to run ‘coho nightly’: 
https://builds.apache.org/view/All/job/cordova-nightly

There were 2 issues:
1. job was using node label that doesn’t have any nodes attached – it has been 
changed by infra at the mid of November
2. NPM credentials, we were using to publish nightlies (user apachebuilds, 
https://www.npmjs.com/~apachebuilds) are now incorrect, at least I’m not able 
to login to NPM with these credentials.

I have updated the job to run on proper set of nodes, but until we get new 
credentials (or at least valid token) we’re blocked on this.

-
Best regards, Vladimir

On 2/15/17, 03:58, "Filip Maj"  wrote:

I recall Alex saying he enabled parallel builds on cloudapp within the
last week, sounds like it might be relevant. We'll need our MSFT
friends and cloudapp admins to rescue us in this situation!

On Tue, Feb 14, 2017 at 4:12 PM, Shazron  wrote:
> I believe they don't run anymore. The last one was on
> 2016-12-09T03:21:01.541Z
>
> Does anyone know what happened?

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[ANNOUNCE] Cordova-windows@5.0.0 released!

2017-02-13 Thread Vladimir Kotikov (Akvelon)

Check out blog at 
https://cordova.apache.org/announcements/2017/02/13/cordova-windows-5.0.0.html
Retweet https://twitter.com/apachecordova/status/831064944208728065

-
Best regards, Vladimir


-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: [Vote] 5.0.0 Windows Release

2017-02-09 Thread Vladimir Kotikov (Akvelon)

I vote +1 

- Verified licenses and license headers
- Ran automates tests
- Verified archive
- Verified tag

-
Best regards, Vladimir
 

On 2/9/17, 09:57, "alsoro...@apache.org"  wrote:

I vote +1.

* Verified archives via `coho verify-archive`
* Verified tags manually
* Verified that blank app creates correctly with platform
* Verified that blank app can be successfully built and ran
* Verified that platform can be updated from previous version
* Verified compatibility with core plugins
* Verified release notes
* Verified version
* Verified line breaks
* Verified compatibility with Visual Studio

-- Alexander Sorokin

-Original Message-
From: dase...@apache.org [mailto:dase...@apache.org] 
Sent: Wednesday, February 8, 2017 8:20 PM
To: dev@cordova.apache.org
Subject: [Vote] 5.0.0 Windows Release

Please review and vote on this 5.0.0 Windows Release

by replying to this email (and keep discussion on the DISCUSS thread)

 

Release issue:
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apac
he.org%2Fjira%2Fbrowse%2FCB-12404=02%7C01%7Cv-alsoro%40microsoft.com%7C
27147ccdfdcc4e8d12ab08d45046afec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%
7C636221711911814136=Q%2BsHad7YDUJ7gaAe4efeD1d1ntNWQNZ6vWPTB9otP6A%3D&
reserved=0

 

The archive has been published to dist/dev:

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache
.org%2Frepos%2Fdist%2Fdev%2Fcordova%2FCB-12404%2F=02%7C01%7Cv-alsoro%40
microsoft.com%7C27147ccdfdcc4e8d12ab08d45046afec%7C72f988bf86f141af91ab2d7cd
011db47%7C1%7C0%7C636221711911814136=EBwPD3j4cb9idteUka7BZzpVVPgXZfkE9
NhTnRbsuDk%3D=0

 

The package was published from its corresponding git tag:

cordova-windows: 5.0.0 (75d7bcf01e)

 

Note that you can test it out via:

 

cordova platform add
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%
2Fapache%2Fcordova-windows%235.0.0=02%7C01%7Cv-alsoro%40microsoft.com%7
C27147ccdfdcc4e8d12ab08d45046afec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0
%7C636221711911814136=2nyElr3rLRACU1mp%2B9CeCepjep8%2BTERrLaZWCZSdX%2B
0%3D=0

 

Upon a successful vote I will upload the archive to dist/, publish it to
npm, and post the blog post.

 

Voting guidelines:
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%
2Fapache%2Fcordova-coho%2Fblob%2Fmaster%2Fdocs%2Frelease-voting.md=02%7
C01%7Cv-alsoro%40microsoft.com%7C27147ccdfdcc4e8d12ab08d45046afec%7C72f988bf
86f141af91ab2d7cd011db47%7C1%7C0%7C636221711911814136=wZ3BZYdWUF7%2BB0
PzVrlR%2B1dQAd0LcsL%2Bxa9nQXqY2wo%3D=0

 

Voting will go on for a minimum of 48 hours.

 

I vote +1:

* Ran coho audit-license-headers over the relevant repos

* Ran coho check-license to ensure all dependencies and subdependencies have
Apache-compatible licenses

* Ensured continuous build was green when repo was tagged

* Ensured that all tests pass

* Created, built and ran mobilespec app

* Tested create and update scenarios via platform scripts and cordova-cli

 

Please let me know if you have any questions or considerations.

 

Best regards,

Sergey Shakhnazarov.

 



-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs issue #681: CB-12404 Add windows@5.0.0 release blog post

2017-01-30 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-docs/pull/681
  
Looks good to me


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #224: Added missing license to PluginInfo.js

2017-01-27 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/224
  
:shipit: 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #213: CB-12163 Make resource-file copy files again

2017-01-25 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/213
  
@ktop, done!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #222: Upgrade cordova-common to 2.0.0

2017-01-25 Thread vladimir-kotikov

GitHub user vladimir-kotikov opened a pull request:

https://github.com/apache/cordova-windows/pull/222

Upgrade cordova-common to 2.0.0



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vladimir-kotikov/cordova-windows master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-windows/pull/222.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #222


commit 15a54f0a59c4f8043801c66f95bf8e74c5b81ce6
Author: Vladimir Kotikov <kotikov.vladi...@gmail.com>
Date:   2017-01-25T14:43:33Z

Upgrade cordova-common to 2.0.0




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib issue #515: CB-12383 Fix cordova_plugins metadata definition

2017-01-25 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-lib/pull/515
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #515: CB-12383 Fix cordova_plugins metadata definit...

2017-01-25 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/515#discussion_r97743108
  
--- Diff: cordova-lib/src/plugman/browserify.js ---
@@ -157,8 +157,8 @@ module.exports = function doBrowserify (project, 
platformApi, pluginInfoProvider
 // instead of generating intermediate file on FS
 var cordova_plugins = new Readable();
 cordova_plugins.push(
-'module.exports.metadata = ' + JSON.stringify(pluginMetadata, 
null, 4) + ';\n' +
-'module.exports = ' + JSON.stringify(modulesMetadata, null, 4) 
+ ';\n', 'utf8');
+'module.exports = ' + JSON.stringify(modulesMetadata, null, 4) 
+ ';\n' +
+'module.exports.metadata = ' + JSON.stringify(pluginMetadata, 
null, 4) + ';\n', 'utf8');
--- End diff --

Cool, thanks for clarification


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #515: CB-12383 Fix cordova_plugins metadata definit...

2017-01-25 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/515#discussion_r97738068
  
--- Diff: cordova-lib/src/plugman/browserify.js ---
@@ -157,8 +157,8 @@ module.exports = function doBrowserify (project, 
platformApi, pluginInfoProvider
 // instead of generating intermediate file on FS
 var cordova_plugins = new Readable();
 cordova_plugins.push(
-'module.exports.metadata = ' + JSON.stringify(pluginMetadata, 
null, 4) + ';\n' +
-'module.exports = ' + JSON.stringify(modulesMetadata, null, 4) 
+ ';\n', 'utf8');
+'module.exports = ' + JSON.stringify(modulesMetadata, null, 4) 
+ ';\n' +
+'module.exports.metadata = ' + JSON.stringify(pluginMetadata, 
null, 4) + ';\n', 'utf8');
--- End diff --

So, if I understand correctly, the whole `module.exports` object was being 
rewritten, so `metadata` property was being lost, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-file-transfer issue #171: CB-12336 (android) Do not call win ...

2017-01-20 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-file-transfer/pull/171
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-inappbrowser issue #208: CB-12353 Corrected merges usage in p...

2017-01-20 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-inappbrowser/pull/208
  
@daserge, should we really remove windows8 section?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-splashscreen issue #123: CB-12369: Add plugin typings from De...

2017-01-20 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-splashscreen/pull/123
  
Travis failure is unrelevant to this change, so merging. However I found 
that native tests are passing with cordova-ios@4.3.0 and failing w/ 4.3.1

/cc @shazron, @daserge 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-splashscreen issue #122: CB-12353 Corrected merges usage in p...

2017-01-18 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-splashscreen/pull/122
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-inappbrowser issue #208: CB-12353 Corrected merges usage in p...

2017-01-18 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-inappbrowser/pull/208
  
LGTM ð 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #220: CB-12298 [Windows] bundle.appxupload not generat...

2017-01-18 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/220
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs issue #674: CB-8486 Corrected the docs on signing

2017-01-18 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-docs/pull/674
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib issue #468: CB-8978 Add resource-file parsing to config.xml

2017-01-16 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-lib/pull/468
  
LGTM ð 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-cli issue #265: CB-12018 : updated tests to function with jasmine in...

2017-01-11 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-cli/pull/265
  
Yeah, not a big deal and probably not worth renaming all test cases back 
now, It just reminded me file transfer tests where we have 'spec.11' then 
`spec.9` then `spec.10`, etc. :) 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-globalization issue #54: CB-11154 (Windows) Return IANA timez...

2017-01-11 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-globalization/pull/54
  
ð¢  it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-cli pull request #265: CB-12018 : updated tests to function with jas...

2017-01-10 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/265#discussion_r95522000
  
--- Diff: spec/cli.spec.js ---
@@ -55,76 +55,76 @@ describe("cordova cli", function () {
 });
 
 describe("options", function () {
-describe("version", function () {
-var version = require("../package").version;
+  describe("version", function () {
--- End diff --

nit: could you please use 4 spaces for indentation to be consistent with 
the rest of the tests/code?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-cli pull request #265: CB-12018 : updated tests to function with jas...

2017-01-10 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/265#discussion_r95521991
  
--- Diff: package.json ---
@@ -11,8 +11,8 @@
 "cordova": "./bin/cordova"
   },
   "scripts": {
-"test": "node node_modules/jasmine-node/bin/jasmine-node 
--captureExceptions --color spec",
-"cover": "node node_modules/istanbul/lib/cli.js cover --root src 
--print detail node_modules/jasmine-node/bin/jasmine-node -- spec"
+"test": "jasmine --captureExceptions --color",
+"cover": "jasmine"
--- End diff --

Does Jasmine generate coverage reports? Shouldn't this line be the same as 
[in 
apache/cordova-lib#510](https://github.com/apache/cordova-lib/pull/510/files#diff-cc6b3b84bb015909917e7c201a31f65aR23)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[DISCUSS] Include TypeScript definitions in plugins

2017-01-09 Thread Vladimir Kotikov (Akvelon)

Hey, everybody!

I’d like to propose/discuss the idea of redistributing Typescript definitions 
along with core plugins, so that users who write their apps in Typescript would 
get the typings in their projects without additional mess with 'tsd'/'typings' 
(these all are the package managers for typescript declarations) or manual 
installation from '@types' NPM org.

As mentioned above, our main goal - to reduce the number of additional actions 
needed to either add the plugin to Typescript project or get the types 
information and intellisense for JavaScript projects. Also, this would reduce 
the number of network calls to other services (typings registry, NPM registry) 
which are known as common points of denial (per telemetry data).

The changes are pretty minimal and include adding a d.ts file with type 
definitions (taken from DefinitelyTyped[1]) and 'types' entry to package.json 
according to Typescript convention [2]. The sample implementation for camera 
plugin is here: 
https://github.com/apache/cordova-plugin-camera/compare/master...vladimir-kotikov:add-typings

Does anyone have any considerations/objections about this proposal?

-
[1] 
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/cordova-plugin-camera/index.d.ts
[2] 
http://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html

-
Best regards, Vladimir



-
Best regards, Vladimir

Re: plugins and engines

2017-01-04 Thread Vladimir Kotikov (Akvelon)

Here is another relevant proposal and discussion: 
https://github.com/cordova/cordova-discuss/pull/30. I think we’ve agreed to 
follow this way to “pin” specific plugins’ versions to specific cordova 
versions.

Here are some examples:
1. 
https://github.com/apache/cordova-plugin-inappbrowser/blob/master/package.json#L47-L54
2. 
https://github.com/litehelpers/Cordova-sqlite-storage/blob/storage-master/package.json#L13-L17

-
Best regards, Vladimir
 

On 1/4/17, 19:21, "julio cesar sanchez"  wrote:

I think we should start testing plugins with cordova-android 4.1.1 as is
the lower required by Google to publish on Google play. If some plugin
doesn't compile then increase the engine version to next cordova-android.
In example, camera plugin doesn't compile with cordova-android 4.1.1.

For cordova-ios we should require at least 3.4.1 as is the version that
included the 64bit support, required by apple, not sure if they require a
newer version for some other reason now.


El 4 ene. 2017 2:52 p. m., "Filip Maj"  escribió:

> Sounds like a good idea, but how to go about doing it? We probably
> can't easily, for example, rule out older versions of iOS without
> someone testing with an old Xcode version.
>
> On Tue, Jan 3, 2017 at 5:15 PM, Shazron  wrote:
> > Related: 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCB-6582=02%7C01%7Cv-vlkoti%40microsoft.com%7Ce2cfd36143c74c85e24408d434bdb765%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636191436803774697=Ny5sFt0LDFmoCUyQbYW1%2B%2Flf38Sz2QKpfuFLqNO3aRE%3D=0
> > (almost 3 years old...)
> >
> > TLDR; we should update the engine tags, with as much granularity as
> > possible.
> >
> > I think we didn't do this because we don't actually know if it *doesn't*
> > work on an older version (since of course we don't test the current
> version
> > with older platform version) and didn't want to unnecessarily restrict a
> > user from installing it.
> >
> > We planned to pin core plugins to a cordova-lib version but we decided 
to
> > use engine tags in plugins:
> > 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcordova%2Fcordova-discuss%2Fblob%2Fmaster%2Fproposals%2F=02%7C01%7Cv-vlkoti%40microsoft.com%7Ce2cfd36143c74c85e24408d434bdb765%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636191436803774697=2bK2SeDLFC50%2Boo4crA%2Fvj3PqRRcZ0iNcRCOzdcpNvU%3D=0
> pinningAndVersioning.md
> >
> >
> > On Tue, Jan 3, 2017 at 12:26 PM, julio cesar sanchez <
> jcesarmob...@gmail.com
> >> wrote:
> >
> >> I have noticed that most of the plugins don't use the engine tags or
> have
> >> them set to cordova 3.0.0 or 3.1.0 which are very old.
> >>
> >> As we drop support for old iOS/Android versions when updating
> cordova-ios
> >> and cordova-android, what is our policy for iOS/Android versions
> support in
> >> plugins?
> >>
> >> Right now people can use the plugins on very old versions of iOS or
> Android
> >> despite we don't support them on the platforms, as the plugins engines
> are
> >> set to 3.0.0 or 3.1.0 on most of them.
> >>
> >> Should we start updating the engines to newer cordova versions? or even
> >> fine grain it to cordova-ios/cordova-android versions?
> >> I have noticed that we even have engines for iOS versions using
> apple-ios
> >> on the engine tag
> >> 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-plugin-wkwebview-=02%7C01%7Cv-vlkoti%40microsoft.com%7Ce2cfd36143c74c85e24408d434bdb765%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636191436803774697=yLXXSTq5hXfxS1yMhOePCv%2F7xLuxpUyzOoTATnUU3bo%3D=0
> >> engine/blob/master/plugin.xml#L35
> >> (but not sure if this really does something as the plugin can be
> >> installed/used in older iOS versions and what works or doesn't work is
> >> controlled in the code)
> >>
> >> Or just say that the old Android/iOS version is not supported by 
Cordova
> >> anymore if someone complains about a plugin not working?
> >>
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>
>

[GitHub] cordova-windows issue #219: CB-12189: Add support for WinMD and DLL combinat...

2016-12-23 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/219
  
Rebased on top of master and force-pushed, will merge once tests pass


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: [VOTE] cordova-serve 1.0.1 release

2016-12-22 Thread Vladimir Kotikov (Akvelon)

I vote +1

- Verified archive signature
- Verified license headers
- Verified dependencies have appropriate licenses
- Verified ‘cordova serve’ command and browser platform work with the version 
being released

-
Best regards, Vladimir
 

On 12/22/16, 13:07, "Alexander Sorokin (Akvelon)"  
wrote:

I vote +1.

* Verified archive with coho
* Verified tag
* Ability to install/uninstall Cordova
* Ability to update Cordova
* Ability to create blank app for Windows, Android
* Ability to build/run apps
* Reviewed release notes
* Verified version
* Verified line breaks
* Verified 'cordova serve'
* Verified that browserified app builds and runs correctly
* Ensured --nobuild option works



From: Tim Barham 
Sent: Thursday, December 22, 2016 7:24:23 AM
To: dev@cordova.apache.org
Subject: [VOTE] cordova-serve 1.0.1 release

Please review and vote on this Tools Release by replying to this email (and 
keep discussion on the DISCUSS thread)

Release issue: 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCB-12289=02%7C01%7Cv-alsoro%40microsoft.com%7C9688f4a40c104c8857de08d42a2279c1%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636179774936024514=Lt701h2YtQAVLUhXv6kqLK2FH5E0X3i245dEIcBGQPk%3D=0

The release has been published to dist/dev: 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdist.apache.org%2Frepos%2Fdist%2Fdev%2Fcordova%2FCB-12289%2F=02%7C01%7Cv-alsoro%40microsoft.com%7C9688f4a40c104c8857de08d42a2279c1%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636179774936024514=mgEMD%2FAXvtRhefGha89Ca%2F1ShQt%2F0wVRe2avrB6vi80%3D=0.
 It has also been published to npm with the "rc" tag, and can be installed for 
testing using "cordova-serve@rc"

The packages were published from their corresponding git tags:

cordova-lib: serve-1.0.1 (510a698841)

Upon a successful vote I will upload the archives to dist/, publish them to 
NPM, and post the corresponding blog post.

Voting guidelines: 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-coho%2Fblob%2Fmaster%2Fdocs%2Frelease-voting.md=02%7C01%7Cv-alsoro%40microsoft.com%7C9688f4a40c104c8857de08d42a2279c1%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636179774936024514=x4H9lVx8d6Q5H%2FSocZjdkHsMu7%2BF0ttejlPH8sVJHbw%3D=0

Voting will go on for a minimum of 48 hours.

I vote +1:

* Ran coho audit-license-headers over the relevant repos
* Ran coho check-license to ensure all dependencies and subdependencies 
have Apache-compatible licenses
* Manually verified package contents

[GitHub] cordova-windows issue #219: CB-12189: Add support for WinMD and DLL combinat...

2016-12-21 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/219
  
@daserge could you take a look please?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #219: Add support for WinMD and DLL combination

2016-12-21 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/219
  
Documentation update: https://github.com/apache/cordova-docs/pull/671
Change to support `implementation` attribute in `cordova-common`: 
https://github.com/apache/cordova-lib/pull/513

Note - this PR has own override for `PluginInfo.getFrameworks` method to 
not depend on not-yet-released apache/cordova-lib#513 - this logic can be 
safely removed once updated `cordova-common` released


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib issue #513: CB-12189 windows: Add `implementation` attribute to ...

2016-12-21 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-lib/pull/513
  
Travis build failures look irrelevant to this PR


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs pull request #671: CB-12189 windows: Document `implementation` ...

2016-12-21 Thread vladimir-kotikov

GitHub user vladimir-kotikov opened a pull request:

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

CB-12189 windows: Document `implementation` attribute



### Platforms affected


### What does this PR do?


### What testing has been done on this change?


### Checklist
- [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vladimir-kotikov/cordova-docs CB-12189

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-docs/pull/671.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #671


commit d2c53d80bf240b1f416dd13483f44d86b82650db
Author: Vladimir Kotikov <kotikov.vladi...@gmail.com>
Date:   2016-12-21T11:09:29Z

CB-12189 Document `implementation` attribute




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request #513: CB-12189 windows: Add `implementation` attrib...

2016-12-21 Thread vladimir-kotikov

GitHub user vladimir-kotikov opened a pull request:

https://github.com/apache/cordova-lib/pull/513

CB-12189 windows: Add `implementation` attribute to frameworks

The attribute is windows-specific and allows to specify implementation for 
WinMD components, written in C++



### Platforms affected


### What does this PR do?


### What testing has been done on this change?


### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [X] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.
- [ ] Added automated test coverage as appropriate for this change.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vladimir-kotikov/cordova-lib CB-12189

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-lib/pull/513.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #513


commit 5be6a8166fd9785dec095c99836cba725fb4947e
Author: Vladimir Kotikov <kotikov.vladi...@gmail.com>
Date:   2016-12-21T12:00:57Z

CB-12189 Add implementation attribute to framework

The attribute is windows-specific and allows to specify implementation for 
WinMD components, written in C++




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #218: CB-12238 [Windows] Colorize titlebar to match sp...

2016-12-20 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/218
  
Sure, ð¢  it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #216: CB-11177 SplashScreen gets shifted on Windows de...

2016-12-19 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/216
  
Yep ð 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs pull request #668: CB-12239 (windows) Add documentation about b...

2016-12-16 Thread vladimir-kotikov

Github user vladimir-kotikov closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs issue #668: CB-12239 (windows) Add documentation about build fl...

2016-12-16 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-docs/pull/668
  
This has been merged in 
https://github.com/apache/cordova-docs/commit/bedbcc1f42a57fd1bd5f71c4dba9010faf1aae07


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #217: CB-12239 Add buildFlag option similar to iOS

2016-12-16 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/217
  
Cool! Merging...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #217: CB-12239 Add buildFlag option similar to iOS

2016-12-16 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/217
  
@daserge, is this good to go?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #218: CB-12238 [Windows] Colorize titlebar to m...

2016-12-15 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/218#discussion_r92604767
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -203,10 +212,30 @@ function enableUserInteraction() {
 document.body.style['-ms-content-zooming'] = origZooming;
 }
 
+// Make title bg color match splashscreen bg color
+function colorizeTitleBar() {
+var appView = 
Windows.UI.ViewManagement.ApplicationView.getForCurrentView();
+if (appView.titleBar) {
+titleInitialBgColor = appView.titleBar.backgroundColor;
+
+appView.titleBar.backgroundColor = titleBgColor;
+appView.titleBar.buttonBackgroundColor = titleBgColor;
+}
+}
+
+// Revert title bg color
+function revertTitleBarColor() {
+var appView = 
Windows.UI.ViewManagement.ApplicationView.getForCurrentView();
+if (appView.titleBar) {
--- End diff --

Is this guaranteed that title bar would exist when you're leaving 
fullscreen? otherwise you may end up not reverting title bar color due to this 
condition. I'm mostly speculating and trying to find really edge cases, but 
still.
Perhaps might be better to subscribe to 
[`ApplicationView.VisibleBoundsChanged`](https://msdn.microsoft.com/en-us/library/windows/apps/windows.ui.viewmanagement.applicationview.visibleboundschanged.aspx)
 and revert the color in the handler?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #216: CB-11177 SplashScreen gets shifted on Win...

2016-12-15 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/216#discussion_r92601539
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -30,7 +30,8 @@ var schema = (isHosted || isWin10UWP && isMsAppxWeb) ? 
'ms-appx-web' : 'ms-appx'
 var fileName = isPhone ? 'splashscreenphone.png' : 'splashscreen.png';
 var splashImageSrc = schema + ':///images/' + fileName;
 
-var splashElement = null, extendedSplashImage = null, 
extendedSplashProgress = null;
+var splashElement = null, extendedSplashImage = null, 
extendedSplashProgress = null,
--- End diff --

nit: please put each definition on its own line - IMO it's be much easier 
to read


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #216: CB-11177 SplashScreen gets shifted on Win...

2016-12-15 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/216#discussion_r92603101
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -148,29 +152,27 @@ function init(config, manifest) {
 extendedSplashProgress.classList.add('win-medium');
 extendedSplashProgress.classList.add('win-ring');
 
+extendedSplashImage.style.maxWidth = "100%";
+extendedSplashImage.style.maxHeight = "100%";
+extendedSplashImage.src = splashImageSrc;
+
 if (isWp81 || isWp10) {
-extendedSplashImage.style.maxWidth = "100%";
-extendedSplashImage.style.maxHeight = "100%";
-extendedSplashImage.src = splashImageSrc;
 // center horizontally
 extendedSplashImage.style.margin = "0 auto";
-extendedSplashImage.style.display = "block";
+extendedSplashImage.style.display = "inline-block";
--- End diff --

Can we move this to CSS using the same technique as for 
`extendedSplashProgress` (additional css classes based on whether we're running 
on desktop/phone)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #216: CB-11177 SplashScreen gets shifted on Win...

2016-12-15 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/216#discussion_r92602532
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -148,29 +152,27 @@ function init(config, manifest) {
 extendedSplashProgress.classList.add('win-medium');
 extendedSplashProgress.classList.add('win-ring');
 
+extendedSplashImage.style.maxWidth = "100%";
--- End diff --

Could you please explain, what's the point in having `maxWidth` and 
`maxHeight`  defined twice - here and in css file?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #216: CB-11177 SplashScreen gets shifted on Win...

2016-12-15 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/216#discussion_r92601889
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -148,29 +152,27 @@ function init(config, manifest) {
 extendedSplashProgress.classList.add('win-medium');
 extendedSplashProgress.classList.add('win-ring');
 
+extendedSplashImage.style.maxWidth = "100%";
+extendedSplashImage.style.maxHeight = "100%";
+extendedSplashImage.src = splashImageSrc;
+
 if (isWp81 || isWp10) {
--- End diff --

nit: Maybe change this condition to `if (isPhoneDevice) {...` where 
`isPhoneDevice = isWp81 || isWp10` - just to improve readability


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #213: CB-12163 Make resource-file copy files again

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/213
  
Looks good ð 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs pull request #668: CB-12239 (windows) Add documentation about b...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-docs/pull/668#discussion_r92459442
  
--- Diff: www/docs/en/dev/guide/platforms/win8/index.md ---
@@ -287,6 +287,37 @@ powershell -Command " & {dir -path 
cert:\LocalMachine\My | where { $_.Subject -l
 
 Once these final values are provided. Cordova should successfully package 
and sign the app.
 
+## MSBuild build flags
+
+Similar to other platforms ([`--gradleArg` on 
Android](../android/index.html#setting-gradle-properties), [`--buildFlag` on 
iOS](../ios/index.html#xcode-build-flags)) you can pass custom flags to 
MSBuild. To do this you have two options:
+
+- add one or more `--buildFlag` options to `cordova build windows` or 
`cordova run windows` commands:
+
+  ```
+  cordova build windows -- --buildFlag /clp:Verbosity=normal 
--buildFlag /p:myCustomProperty=Value
--- End diff --

I think there is no big difference since `nopt`, which we use under the 
hood, understands both.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #217: CB-12239 Add buildFlag option similar to ...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/217#discussion_r92399420
  
--- Diff: template/cordova/lib/MSBuildTools.js ---
@@ -46,11 +46,8 @@ MSBuildTools.prototype.buildProject = function(projFile, 
buildType, buildarch, o
 '/p:Configuration=' + buildType,
 '/p:Platform=' + buildarch];
 
-if (otherConfigProperties) {
-var keys = Object.keys(otherConfigProperties);
-keys.forEach(function(key) {
-args.push('/p:' + key + '=' + otherConfigProperties[key]);
--- End diff --

Also it's expected that user should provide already prefixed flags (since 
`/p:` is not the only prefix available)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #217: CB-12239 Add buildFlag option similar to ...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/217#discussion_r92399214
  
--- Diff: template/cordova/lib/MSBuildTools.js ---
@@ -46,11 +46,8 @@ MSBuildTools.prototype.buildProject = function(projFile, 
buildType, buildarch, o
 '/p:Configuration=' + buildType,
 '/p:Platform=' + buildarch];
 
-if (otherConfigProperties) {
-var keys = Object.keys(otherConfigProperties);
-keys.forEach(function(key) {
-args.push('/p:' + key + '=' + otherConfigProperties[key]);
--- End diff --

The former one. Please see 
[build.js:344](https://github.com/apache/cordova-windows/pull/217/files/6308e6974f5499857254fb79c91ded422b46486f#diff-9d172d6844120225b6cfb763491e4e03R344)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #213: CB-12163 Make resource-file copy files ag...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/213#discussion_r92344562
  
--- Diff: template/cordova/lib/JsprojManager.js ---
@@ -121,7 +121,12 @@ jsprojManager.prototype = {
 copyToOutputDirectory.text = 'Always';
 children.push(copyToOutputDirectory);
 
-var item = createItemGroupElement('ItemGroup/Content', sourcePath, 
targetConditions, children);
+var item;
--- End diff --

IMO it'd be better to calculate path for `content` element outside this 
method (in `'resource-file'.install`) - this way you will avoid checking the 
same condition (`if (targetConditions.reference) ...`) twice and won't need to 
modify this method at all.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #213: CB-12163 Make resource-file copy files ag...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/213#discussion_r92345318
  
--- Diff: template/cordova/lib/PluginHandler.js ---
@@ -188,7 +207,7 @@ module.exports.getUninstaller = function(type) {
 };
 
 function getTargetConditions(obj) {
-return { versions: obj.versions, deviceTarget: obj.deviceTarget, arch: 
obj.arch };
+return { versions: obj.versions, deviceTarget: obj.deviceTarget, arch: 
obj.arch, reference: obj.reference };
--- End diff --

As per comments above `reference` field is not really required - i's easier 
to use `obj.reference` directly


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #213: CB-12163 Make resource-file copy files ag...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/213#discussion_r92344997
  
--- Diff: template/cordova/lib/PluginHandler.js ---
@@ -53,18 +53,37 @@ var handlers = {
 },
 'resource-file':{
 install:function(obj, plugin, project, options) {
-// do not copy, but reference the file in the plugin folder. 
This allows to
-// have multiple source files map to the same target and 
select the appropriate
-// one based on the current build settings, e.g. architecture.
-// also, we don't check for existence. This allows to insert 
build variables
-// into the source file name, e.g.
-// 
-var relativeSrcPath = getPluginFilePath(plugin, obj.src, 
project.projectFolder);
-project.addResourceFileToProject(relativeSrcPath, obj.target, 
getTargetConditions(obj));
+var targetConditions = getTargetConditions(obj);
+if (targetConditions.reference) {
+// do not copy, but reference the file in the plugin 
folder. This allows to
+// have multiple source files map to the same target and 
select the appropriate
+// one based on the current build settings, e.g. 
architecture.
+// also, we don't check for existence. This allows to 
insert build variables
+// into the source file name, e.g.
+// 
+var relativeSrcPath = getPluginFilePath(plugin, obj.src, 
project.projectFolder);
+project.addResourceFileToProject(relativeSrcPath, 
obj.target, targetConditions);
+} else {
+// if target already exists, emit warning to consider 
using a reference instead of copying
+if (fs.existsSync(path.resolve(project.root, obj.target))) 
{
+events.emit('warn', ' with target ' + 
obj.target + ' already exists and will be overwritten ' +
+'by a  with the same target. Consider 
using the attribute reference="true" in the ' +
+' tag to avoid overwriting files with 
the same target. ');
--- End diff --

Might be also useful to add that adding `reference` will not copy user 
files to destination directory. WDYT?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #213: CB-12163 Make resource-file copy files ag...

2016-12-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/213#discussion_r92344993
  
--- Diff: template/cordova/lib/PluginHandler.js ---
@@ -53,18 +53,37 @@ var handlers = {
 },
 'resource-file':{
 install:function(obj, plugin, project, options) {
-// do not copy, but reference the file in the plugin folder. 
This allows to
-// have multiple source files map to the same target and 
select the appropriate
-// one based on the current build settings, e.g. architecture.
-// also, we don't check for existence. This allows to insert 
build variables
-// into the source file name, e.g.
-// 
-var relativeSrcPath = getPluginFilePath(plugin, obj.src, 
project.projectFolder);
-project.addResourceFileToProject(relativeSrcPath, obj.target, 
getTargetConditions(obj));
+var targetConditions = getTargetConditions(obj);
+if (targetConditions.reference) {
--- End diff --

I think that the object that `getTargetConditions` returns was originally 
meant to hold details about target platform (Win8.1, WP8.1, Win10) and 
architecture to create a `condition` attribute, so `reference` field  doesn't 
really fit here - you might just check `if (obj.reference) ...`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #217: CB-12239 Add buildFlag option similar to iOS

2016-12-13 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/217
  
Pls also see corresponding documentation update here: 
https://github.com/apache/cordova-docs/pull/668


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-docs pull request #668: CB-12239 (windows) Add documentation about b...

2016-12-13 Thread vladimir-kotikov

GitHub user vladimir-kotikov opened a pull request:

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

CB-12239 (windows) Add documentation about build flags for Windows



### Platforms affected

Windows

### What does this PR do?

Documents use of `buildFlag` option

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.


![image](https://cloud.githubusercontent.com/assets/3857604/21137756/0ce2c024-c13c-11e6-9e8d-43243de1559e.png)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vladimir-kotikov/cordova-docs CB-12239

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-docs/pull/668.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #668


commit 9fc8b3aafaf75091ce03aa68d474f7bd7943d517
Author: Vladimir Kotikov <kotikov.vladi...@gmail.com>
Date:   2016-12-06T14:58:31Z

CB-12239 Add documentation about build flags for Windows




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #217: CB-12239 Add buildFlag option similar to iOS

2016-12-13 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/217
  
@TimBarham, @alsorokin, @daserge, could you please take a look?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #217: CB-12239 Add buildFlag option similar to ...

2016-12-13 Thread vladimir-kotikov

GitHub user vladimir-kotikov opened a pull request:

https://github.com/apache/cordova-windows/pull/217

CB-12239 Add buildFlag option similar to iOS



### What does this PR do?

This PR adds `buildFlag` CLI argument and corresponding option for 
`build.json` file (similar to iOS) to allow users to pass arbitrary options 
directly to MSBuild


### What testing has been done on this change?

Added new tests, ran unit and e2e tests

### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) 
in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with 
resolving file paths", where CB- is the JIRA ID & "android" is the platform 
affected.
- [x] Added automated test coverage as appropriate for this change.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vladimir-kotikov/cordova-windows 
build-flags-support

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/cordova-windows/pull/217.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #217


commit 6308e6974f5499857254fb79c91ded422b46486f
Author: Vladimir Kotikov <kotikov.vladi...@gmail.com>
Date:   2016-12-12T15:45:51Z

CB-12239 Add buildFlag option similar to iOS




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: [DISCUSS] EOL cordova-medic

2016-12-12 Thread Vladimir Kotikov (Akvelon)

Hey all!

I’m +1 for killing Medic, since it hasn’t been used for months and I guess 
hardly ever will be used in the future.

Though I have a bit different proposal about moving configs – IMO they are a 
part of CI and should live somewhere inside a CI. I don’t think there is a need 
to either exhibit them to public or make them easily editable by anybody – the 
current practice demonstrates that the only one who really needs to see or edit 
these configs is the CI maintainer. From the implementation side, I think we 
can just reuse one of the millions of Jenkins plugins (maybe this: 
https://wiki.jenkins-ci.org/display/JENKINS/Config+File+Provider+Plugin)

> Substantial code was added to paramedic to support some other use cases 
> (Appium, Sauce, ...), but I would still REALLY like to prevent us turning 
> paramedic into a behemoth travelling hospital ... it may already be too late 
> though.

Jesse, I share your concerns! Just want to say that despite it became really 
complex, and the code is far from ideal, paramedic really does its job!
I wish we could have a bit of spare time and some free hands to redesign it and 
make it lightweight and modular (I think all that added functionality, like 
Appium and Sauce support, easily could be moved out of paramedic into “plugins”)

-
Best regards, Vladimir

On 12/13/16, 01:03, "Filip Maj"  wrote:

On Mon, Dec 12, 2016 at 1:46 PM, Jesse  wrote:
> What would be the added responsibilities for cordova-paramedic?

Good question. As far as I can tell, I believe paramedic would need to
additionally house the helper JSON files containing configuration that
one passes into paramedic - basically replacing the various flags one
can run paramedic with with a single one pointing to a file instead.

In particular, the Jenkins configs that the cloudapp CI uses that it
feeds into paramedic [1] - including the "local" ones used for local
testing [2], which seem particularly relevant to paramedic's mission.
This would save our Jenkins instance from pulling the medic repo down,
speeding up test execution (it seems silly to clone a whole repo just
to pull a few JSON files out!).

So right now, when I want to run paramedic locally, I invoke something like:

$ node cordova-paramedic/main.js --platform android --plugin
./cordova-plugin-contacts --verbose --cleanUpAfterRun

I can reduce on the flags passed in by pointing paramedic to the
afore-mentioned local configs, but, these currently exist in the medic
repo. So I've actually been running paramedic recently like so:

$ node cordova-paramedic/main.js --plugin
./cordova-plugin-contacts --config
./cordova-medic/jenkins-conf/pr/local/android-5.1.config.json

The above incantation is also how our cloudapp CI invokes paramedic:
by pointing it to a config file (it points it to a "non-local" config
file, e.g. [3]).

In effect, it is just shuffling around code from one repo to the
other, but the benefits we get are:
 - save compute within our CI
 - all testing-relevant configurations will exist in one repo, not two
 - we can remove the medic repo along with its code duplication of paramedic

The cons, as far as I can tell, are:
 - there will be a new 'config' top-level directory

Let me know how that sounds and what else I've missed.

[1] 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-medic%2Ftree%2Fmaster%2Fjenkins-conf=02%7C01%7Cv-vlkoti%40microsoft.com%7C2e9fcdbcd93d4516275708d422dab62e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171770130218288=YjWqa5CiRntxkhmikqNE12bwGaEf0W4q2YF%2BT8E1Asg%3D=0
[2] 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-medic%2Ftree%2Fmaster%2Fjenkins-conf%2Fpr%2Flocal=02%7C01%7Cv-vlkoti%40microsoft.com%7C2e9fcdbcd93d4516275708d422dab62e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171770130218288=FbkW3oVkKBw%2FrlPTc3wUCD5d3dRdTYDvYPTj1h%2FUYRM%3D=0
[3] 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-medic%2Fblob%2Fmaster%2Fjenkins-conf%2Fpr%2Fandroid-5.1.config.json=02%7C01%7Cv-vlkoti%40microsoft.com%7C2e9fcdbcd93d4516275708d422dab62e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636171770130218288=BqBTYje0DLo2ND6SPxkLTcgyI1c0HA0qSoXS7%2FhdVw8%3D=0

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-cli pull request #265: CB-12018 : updated tests to function with jas...

2016-12-12 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/265#discussion_r92010506
  
--- Diff: package.json ---
@@ -31,6 +31,7 @@
 "cordova-common": "1.5.x",
 "cordova-lib": "6.4.0",
 "insight": "~0.8.2",
+"jasmine": "^2.5.2",
--- End diff --

Shouldn't this be a dev dependency?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-cli pull request #265: CB-12018 : updated tests to function with jas...

2016-12-12 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-cli/pull/265#discussion_r92009821
  
--- Diff: package.json ---
@@ -11,7 +11,7 @@
 "cordova": "./bin/cordova"
   },
   "scripts": {
-"test": "node node_modules/jasmine-node/bin/jasmine-node 
--captureExceptions --color spec",
+"test": "node node_modules/jasmine/bin/jasmine --captureExceptions 
--color spec",
 "cover": "node node_modules/istanbul/lib/cli.js cover --root src 
--print detail node_modules/jasmine-node/bin/jasmine-node -- spec"
--- End diff --

@audreyso, looks like you'll also need to update `cover` command to use 
jasmine inatead of jasmine-node


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-battery-status issue #46: CB-12227 (windows) Fixed Browserify...

2016-12-12 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-battery-status/pull/46
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: [DISCUSS] Plugins Release

2016-12-12 Thread Vladimir Kotikov (Akvelon)

Hi Shazron
Gave you permissions to packages which I have access to.

-
Best regards, Vladimir
 

On 12/12/16, 01:37, "Shazron"  wrote:

Hi Steve Gill and/or Carlos Santana -- please give me write access to these
npm modules below. Publishing is stalled until I get permissions.

cordova-plugin-battery-status
csantanapr
stevegill
cordova-plugin-camera
csantanapr
stevegill
cordova-plugin-console
csantanapr
stevegill
cordova-plugin-contacts
csantanapr
stevegill
cordova-plugin-device
csantanapr
stevegill
cordova-plugin-device-motion
csantanapr
stevegill
cordova-plugin-device-orientation
csantanapr
stevegill
cordova-plugin-dialogs
csantanapr
stevegill
cordova-plugin-file
kotikov.vladimir
csantanapr
stevegill
cordova-plugin-file-transfer
nikhilkh
csantanapr
stevegill
cordova-plugin-geolocation
csantanapr
stevegill
cordova-plugin-globalization
csantanapr
stevegill
cordova-plugin-legacy-whitelist
csantanapr
stevegill
cordova-plugin-media
kotikov.vladimir
csantanapr
stevegill
cordova-plugin-media-capture
csantanapr
stevegill
cordova-plugin-network-information
csantanapr
stevegill
cordova-plugin-screen-orientation
gbenvenuti
stevegill
tony--
cordova-plugin-splashscreen
kotikov.vladimir
csantanapr
stevegill
cordova-plugin-statusbar
kotikov.vladimir
csantanapr
stevegill
cordova-plugin-test-framework
csantanapr
stevegill
cordova-plugin-vibration
csantanapr
stevegill
cordova-plugin-whitelist
csantanapr
stevegill

--

I already have access to these:

cordova-plugin-wkwebview-engine
bowserj
kotikov.vladimir
purplecabbage
shazron
csantanapr
stevegill
cordova-plugin-inappbrowser
csantanapr
stevegill
kotikov.vladimir
sgrebnov
shazron


On Sun, Dec 11, 2016 at 2:11 PM, Shazron  wrote:

> I've already started 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCB-12237=02%7C01%7Cv-vlkoti%40microsoft.com%7C0e0639e78dac47d771a708d4221658d2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636170926753647122=w%2F0INJNEerZ%2FYkorTQO%2BAd1x%2FTOk5bqtOI6gHW3ZU8w%3D=0
>
> On Sun, Dec 11, 2016 at 2:01 PM, Shazron  wrote:
>
>> One more issue -- cordova-plugin-inappbrowser, my intent is not to call
>> anyone out here (hey I'm the major source of mistakes for this release) 
but
>> my intent is to fix the problem. The plugin minor version was incremented
>> in plugin.xml to 1.6.0 but not incremented in package.json. Thus when the
>> packaging automation ran, the version had a mismatch, and that this is
>> reflected in cordova-plugin-inappbrowser.
>>
>> 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-plugin-inappbrowser%2Fcommit=02%7C01%7Cv-vlkoti%40microsoft.com%7C0e0639e78dac47d771a708d4221658d2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636170926753647122=8B2GLsiiMxL7357w%2BsmrgTDexI2oVBImPKiOVvx9NQ8%3D=0
>> /cfbd3845a893df647ff39ec3c750804d775a0e57
>>
>> We should have a version mismatch automation check during release as well
>> in coho, and/or the tests.
>>
>> So now the problem is, I can't publish cordova-plugin-inappbrowser this
>> release because package.json shows the version is 1.5.1 while plugin.xml 
is
>> 1.6.0 (1.6.0 is correct).
>>
>> I will have to omit cordova-plugin-inappbrowser in this release, and put
>> out another cordova plugins release which includes:
>>
>> 1) Updates in cordova-plugin-battery-status that we discussed
>> 2) The package.json update in cordova-plugin-inappbrowser (same version,
>> 1.6.0)
>>
>>
>>
>>
>>
>>
>>
>> On Sun, Dec 11, 2016 at 11:32 AM, Shazron  wrote:
>>
>>> I screwed up the release notes for each plugin (RELEASENOTES.md) in each
>>> plugin repo. I'll fix it after release, but will fix it for the blog 
post:
>>> 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fissues.apache.org%2Fjira%2Fbrowse%2FCB-12236=02%7C01%7Cv-vlkoti%40microsoft.com%7C0e0639e78dac47d771a708d4221658d2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636170926753647122=2Q%2Bi07ZZetyqGbDkfxXu2CynpfWdzVruLXcy7vNfJMI%3D=0
>>>
>>> On Thu, Dec 8, 2016 at 9:03 PM,  wrote:
>>>
 Just replying to it from my apache address to

[GitHub] cordova-windows issue #212: CB-9287 Not enough Icons and Splashscreens for W...

2016-12-01 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/212
  
LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89992916
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -297,48 +305,80 @@ function applyNavigationWhitelist(config, manifest) {
 manifest.getApplication().setAccessRules(whitelistRules);
 }
 
-function mapImageResources(images, imagesDir) {
-var pathMap = {};
+// Platform default images
+var platformImages = [
+{dest: 'Square150x150Logo.scale-100', width: 150, height: 150},
+{dest: 'Square30x30Logo.scale-100', width: 30, height: 30},
+{dest: 'StoreLogo.scale-100', width: 50, height: 50},
+{dest: 'SplashScreen.scale-100', width: 620, height: 300, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-125', width: 775, height: 375, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-140', width: 868, height: 420, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-150', width: 930, height: 450, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-180', width: 1116, height: 540, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-200', width: 1240, height: 600, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-400', width: 2480, height: 1200, 
targetProject: TARGET_PROJECT_10},
+// scaled images are specified here for backward compatibility only so 
we can find them by size
+{dest: 'StoreLogo.scale-240', width: 120, height: 120},
+{dest: 'Square44x44Logo.scale-100', width: 44, height: 44},
+{dest: 'Square44x44Logo.scale-240', width: 106, height: 106},
+{dest: 'Square70x70Logo.scale-100', width: 70, height: 70},
+{dest: 'Square71x71Logo.scale-100', width: 71, height: 71},
+{dest: 'Square71x71Logo.scale-240', width: 170, height: 170},
+{dest: 'Square150x150Logo.scale-240', width: 360, height: 360},
+{dest: 'Square310x310Logo.scale-100', width: 310, height: 310},
+{dest: 'Wide310x150Logo.scale-100', width: 310, height: 150},
+{dest: 'Wide310x150Logo.scale-240', width: 744, height: 360},
+{dest: 'SplashScreenPhone.scale-100', width: 480, height: 800, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-140', width: 672, height: 1120, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-240', width: 1152, height: 1920, 
targetProject: TARGET_PROJECT_WP81}
+];
+
+function findPlatformImage(width, height) {
+if (!width && !height){
+// this could be default image,
+// Windows requires specific image dimension so we can't apply it
+return null;
+}
+for (var idx in platformImages){
+var res = platformImages[idx];
+// If only one of width or height is not specified, use another 
parameter for comparation
+// If both specified, compare both.
+if ((!width || (width == res.width)) &&
+(!height || (height == res.height))){
+return res;
+}
+}
+return null;
+}
 
-// Platform default images
-var platformImages = [
-{dest: 'Square150x150Logo.scale-100.png', width: 150, height: 150},
-{dest: 'Square30x30Logo.scale-100.png', width: 30, height: 30},
-{dest: 'StoreLogo.scale-100.png', width: 50, height: 50},
-{dest: 'SplashScreen.scale-100.png', width: 620, height: 300},
-// scaled images are specified here for backward compatibility 
only so we can find them by size
-{dest: 'StoreLogo.scale-240.png', width: 120, height: 120},
-{dest: 'Square44x44Logo.scale-100.png', width: 44, height: 44},
-{dest: 'Square44x44Logo.scale-240.png', width: 106, height: 106},
-{dest: 'Square70x70Logo.scale-100.png', width: 70, height: 70},
-{dest: 'Square71x71Logo.scale-100.png', width: 71, height: 71},
-{dest: 'Square71x71Logo.scale-240.png', width: 170, height: 170},
-{dest: 'Square150x150Logo.scale-240.png', width: 360, height: 360},
-{dest: 'Square310x310Logo.scale-100.png', width: 310, height: 310},
-{dest: 'Wide310x150Logo.scale-100.png', width: 310, height: 150},
-{dest: 'Wide310x150Logo.scale-240.png', width: 744, height: 360},
-{dest: 'SplashScreenPhone.scale-240.png', width: 1152, height: 
1920}
-];
+/** Maps MRT splashscreen image to its target project defined in 
platformImages -> 8.1|WP8.1|10
+ * This assumes we have different scales used for 8.1 and 10 projects.
+ * The only intersection is scale-100, which is treated as Win10 project' 
splashscreen (because
+ * size limit applies to Win10 project so we'll need to check it).

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89994632
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +492,107 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/**
+ * Checks if the splash screen matches the target project
+ * @param  {Object} splash SplashScreen object to check
+ * @param  {String} targetName SplashScreen target name: 
'SplashScreen'|'SplashScreenPhone'
+ * @returns {Boolean} True if the splash screen matches the target project
+ */
+function splashScreenTargetIs(splash, targetName) {
+var targetImg;
+
+if (splash.target) {
+// MRT syntax:
+return splash.target === targetName;
+} 
+
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+return 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
--- End diff --

IMO it's not very clear, what is `splashScreenTargetIs` function for and 
what values it can return. I'd suggest to rework this in the following way:
1. have a function that returns a target project based on image `target` or 
dimensions (e.g. `getTargetForImage`)
2. do a comparison of that function's result and `target` variable here so 
it's be clear that we expect result to match target



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89990562
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -58,8 +58,45 @@ function readBoolFromCfg(preferenceName, defaultValue, 
cfg) {
 }
 }
 
+// Refine splashscreen file extension to match what is in config.xml
+function refineSplashScreenExtension(cfg) {
--- End diff --

> Then we'll need to manipulate the Image.src anyway as it is in different 
format.

Why? Isn't the logic in 'prepare' supposed to update splash screen file 
name to actual value? The benefit IMO is that you'll have one place where 
you're trying to infer splashscreen real filename based on `splashscreen` 
preferences


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89991437
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -297,48 +305,80 @@ function applyNavigationWhitelist(config, manifest) {
 manifest.getApplication().setAccessRules(whitelistRules);
 }
 
-function mapImageResources(images, imagesDir) {
-var pathMap = {};
+// Platform default images
+var platformImages = [
--- End diff --

nit: could you please make it uppercase, so i'd be easy to understand in 
code that we're using some constant value


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89993606
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -353,6 +391,13 @@ function mapImageResources(images, imagesDir) {
 events.emit('warn', 'No images found for target: ' + 
img.target);
 } else {
 candidates.forEach(function(mrtImage) {
+if (img.target === SPLASH_SCREEN_DESKTOP_TARGET_NAME &&
+mrtSplashScreenToTargetProject(mrtImage) === 
TARGET_PROJECT_10 &&
+exceedsSizeLimit(mrtImage.path)) {
--- End diff --

Makes sense


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89991151
  
--- Diff: template/cordova/lib/AppxManifest.js ---
@@ -416,6 +417,18 @@ AppxManifest.prototype.getVisualElements = function () 
{
 }
 return this;
 },
+getSplashScreenExtension: function (extension) {
--- End diff --

Instead of very specific to this issue `get/setSplashScreenExtension` can 
we have more generic `get/setSplashScreen` to set the filename?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89991524
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -297,48 +305,80 @@ function applyNavigationWhitelist(config, manifest) {
 manifest.getApplication().setAccessRules(whitelistRules);
 }
 
-function mapImageResources(images, imagesDir) {
-var pathMap = {};
+// Platform default images
+var platformImages = [
+{dest: 'Square150x150Logo.scale-100', width: 150, height: 150},
+{dest: 'Square30x30Logo.scale-100', width: 30, height: 30},
+{dest: 'StoreLogo.scale-100', width: 50, height: 50},
+{dest: 'SplashScreen.scale-100', width: 620, height: 300, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-125', width: 775, height: 375, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-140', width: 868, height: 420, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-150', width: 930, height: 450, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-180', width: 1116, height: 540, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-200', width: 1240, height: 600, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-400', width: 2480, height: 1200, 
targetProject: TARGET_PROJECT_10},
+// scaled images are specified here for backward compatibility only so 
we can find them by size
+{dest: 'StoreLogo.scale-240', width: 120, height: 120},
+{dest: 'Square44x44Logo.scale-100', width: 44, height: 44},
+{dest: 'Square44x44Logo.scale-240', width: 106, height: 106},
+{dest: 'Square70x70Logo.scale-100', width: 70, height: 70},
+{dest: 'Square71x71Logo.scale-100', width: 71, height: 71},
+{dest: 'Square71x71Logo.scale-240', width: 170, height: 170},
+{dest: 'Square150x150Logo.scale-240', width: 360, height: 360},
+{dest: 'Square310x310Logo.scale-100', width: 310, height: 310},
+{dest: 'Wide310x150Logo.scale-100', width: 310, height: 150},
+{dest: 'Wide310x150Logo.scale-240', width: 744, height: 360},
+{dest: 'SplashScreenPhone.scale-100', width: 480, height: 800, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-140', width: 672, height: 1120, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-240', width: 1152, height: 1920, 
targetProject: TARGET_PROJECT_WP81}
+];
+
+function findPlatformImage(width, height) {
+if (!width && !height){
+// this could be default image,
+// Windows requires specific image dimension so we can't apply it
+return null;
+}
+for (var idx in platformImages){
+var res = platformImages[idx];
+// If only one of width or height is not specified, use another 
parameter for comparation
+// If both specified, compare both.
+if ((!width || (width == res.width)) &&
+(!height || (height == res.height))){
+return res;
+}
+}
+return null;
+}
 
-// Platform default images
-var platformImages = [
-{dest: 'Square150x150Logo.scale-100.png', width: 150, height: 150},
-{dest: 'Square30x30Logo.scale-100.png', width: 30, height: 30},
-{dest: 'StoreLogo.scale-100.png', width: 50, height: 50},
-{dest: 'SplashScreen.scale-100.png', width: 620, height: 300},
-// scaled images are specified here for backward compatibility 
only so we can find them by size
-{dest: 'StoreLogo.scale-240.png', width: 120, height: 120},
-{dest: 'Square44x44Logo.scale-100.png', width: 44, height: 44},
-{dest: 'Square44x44Logo.scale-240.png', width: 106, height: 106},
-{dest: 'Square70x70Logo.scale-100.png', width: 70, height: 70},
-{dest: 'Square71x71Logo.scale-100.png', width: 71, height: 71},
-{dest: 'Square71x71Logo.scale-240.png', width: 170, height: 170},
-{dest: 'Square150x150Logo.scale-240.png', width: 360, height: 360},
-{dest: 'Square310x310Logo.scale-100.png', width: 310, height: 310},
-{dest: 'Wide310x150Logo.scale-100.png', width: 310, height: 150},
-{dest: 'Wide310x150Logo.scale-240.png', width: 744, height: 360},
-{dest: 'SplashScreenPhone.scale-240.png', width: 1152, height: 
1920}
-];
+/** Maps MRT splashscreen image to its target project defined in 
platformImages -> 8.1|WP8.1|10
--- End diff --

nit: another JSDoc format issue


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-29 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89996388
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
+} else {
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+matchesTarget = 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+return matchesTarget;
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
+
+if (matchesTarget === true) {
+extensionsUsed[target] = extensionsUsed[target] || [];
+
extensionsUsed[target].push(path.extname(img.src.toLowerCase()));
+}
+
+return matchesTarget;
+}
+
+function updateSplashExtensionInManifest(manifestFileName, 
splashScreen) {
+var manifest = AppxManifest.get(path.join(locations.root, 
manifestFileName));
+var newExtension = path.extname(splashScreen.src);
+
+if (manifest.getVisualElements().getSplashScreenExtension() !== 
newExtension) {
+events.emit('verbose', 'Set ' + manifestFileName + ' 
SplashScreen image extension to "' + newExtension + '"');
+
manifest.getVisualElements().setSplashScreenExtension(newExtension);
+manifest.write();
+}
+}
+
+var splashScreens = 
cordovaProject.projectConfig.getSplashScreens('windows');
+
+var desktopSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_DESKTOP_TARGET_NAME);
+})[0];
+
+var phoneSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_PHONE_TARGET_NAME);
+})[0];
+
+checkThatExtensionsAreNotMixed();
--- End diff --

> You should not mix image extensions for non-MRT syntax inside one project 
group

I thought it's pretty obvious :)

> in this case ^ the second JPG image will not be used as AppxManifest will 
have PNG for SplashScreen.Image' extension

Yep. If you specify the same splascreens using MRT and 'target' attribute, 
`splashscreen.scale-125.jpg` also would be silently ignored because 
`mapImageResources` requires extensions to match. That said I don't think we 
need to care about this at all.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729105
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -353,6 +391,13 @@ function mapImageResources(images, imagesDir) {
 events.emit('warn', 'No images found for target: ' + 
img.target);
 } else {
 candidates.forEach(function(mrtImage) {
+if (img.target === SPLASH_SCREEN_DESKTOP_TARGET_NAME &&
+mrtSplashScreenToTargetProject(mrtImage) === 
TARGET_PROJECT_10 &&
+exceedsSizeLimit(mrtImage.path)) {
--- End diff --

Are you sure that we need to handle this case for the user? I'd rather the 
build fail to clearly indicate to user that spash image must be updated.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89726640
  
--- Diff: spec/unit/Prepare.Win10.spec.js ---
@@ -541,4 +541,165 @@ describe('copyIcons method', function () {
 
expect(FileUpdater.updatePaths).toHaveBeenCalledWith(expectedPathMap, { 
rootDir: PROJECT }, logFileOp);
 });
 });
+
+it('should ignore splashScreens for Windows 10 project with size >200K 
and emit a warning', function () {
+var size300K = 300 * 1024;
+var warnSpy = jasmine.createSpy('warn');
+events.on('warn', warnSpy);
+
+var splashScreensFiles = [
--- End diff --

Nit: this could be calculated from `splashScreens`: 
`splashScreens.map(splash => path.basename(splash.src)`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89726245
  
--- Diff: cordova-js-src/splashscreen.js ---
@@ -58,8 +58,45 @@ function readBoolFromCfg(preferenceName, defaultValue, 
cfg) {
 }
 }
 
+// Refine splashscreen file extension to match what is in config.xml
+function refineSplashScreenExtension(cfg) {
--- End diff --

Just a suggestion - is it possible to read splashscreen file name and 
extension directly from appxmanifest? This way you could avoid all this tricky 
logic for getting the right filename.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729119
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -362,7 +407,13 @@ function mapImageResources(images, imagesDir) {
 // find target image by size
 var targetImg = findPlatformImage(img.width, img.height);
 if (targetImg) {
-var targetPath = path.join(imagesDir, targetImg.dest);
+if (targetImg.targetProject === TARGET_PROJECT_10 &&
--- End diff --

Same as above


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729302
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
--- End diff --

Nit: this is not a valid syntax for multiline JSDoc comments, please update


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89727836
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -297,48 +305,78 @@ function applyNavigationWhitelist(config, manifest) {
 manifest.getApplication().setAccessRules(whitelistRules);
 }
 
-function mapImageResources(images, imagesDir) {
-var pathMap = {};
+// Platform default images
+var platformImages = [
+{dest: 'Square150x150Logo.scale-100', width: 150, height: 150},
+{dest: 'Square30x30Logo.scale-100', width: 30, height: 30},
+{dest: 'StoreLogo.scale-100', width: 50, height: 50},
+{dest: 'SplashScreen.scale-100', width: 620, height: 300, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-125', width: 775, height: 375, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-140', width: 868, height: 420, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-150', width: 930, height: 450, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-180', width: 1116, height: 540, 
targetProject: TARGET_PROJECT_81},
+{dest: 'SplashScreen.scale-200', width: 1240, height: 600, 
targetProject: TARGET_PROJECT_10},
+{dest: 'SplashScreen.scale-400', width: 2480, height: 1200, 
targetProject: TARGET_PROJECT_10},
+// scaled images are specified here for backward compatibility only so 
we can find them by size
+{dest: 'StoreLogo.scale-240', width: 120, height: 120},
+{dest: 'Square44x44Logo.scale-100', width: 44, height: 44},
+{dest: 'Square44x44Logo.scale-240', width: 106, height: 106},
+{dest: 'Square70x70Logo.scale-100', width: 70, height: 70},
+{dest: 'Square71x71Logo.scale-100', width: 71, height: 71},
+{dest: 'Square71x71Logo.scale-240', width: 170, height: 170},
+{dest: 'Square150x150Logo.scale-240', width: 360, height: 360},
+{dest: 'Square310x310Logo.scale-100', width: 310, height: 310},
+{dest: 'Wide310x150Logo.scale-100', width: 310, height: 150},
+{dest: 'Wide310x150Logo.scale-240', width: 744, height: 360},
+{dest: 'SplashScreenPhone.scale-100', width: 480, height: 800, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-140', width: 672, height: 1120, 
targetProject: TARGET_PROJECT_WP81},
+{dest: 'SplashScreenPhone.scale-240', width: 1152, height: 1920, 
targetProject: TARGET_PROJECT_WP81}
+];
+
+function findPlatformImage(width, height) {
+if (!width && !height){
+// this could be default image,
+// Windows requires specific image dimension so we can't apply it
+return null;
+}
+for (var idx in platformImages){
+var res = platformImages[idx];
+// If only one of width or height is not specified, use another 
parameter for comparation
+// If both specified, compare both.
+if ((!width || (width == res.width)) &&
+(!height || (height == res.height))){
+return res;
+}
+}
+return null;
+}
 
-// Platform default images
-var platformImages = [
-{dest: 'Square150x150Logo.scale-100.png', width: 150, height: 150},
-{dest: 'Square30x30Logo.scale-100.png', width: 30, height: 30},
-{dest: 'StoreLogo.scale-100.png', width: 50, height: 50},
-{dest: 'SplashScreen.scale-100.png', width: 620, height: 300},
-// scaled images are specified here for backward compatibility 
only so we can find them by size
-{dest: 'StoreLogo.scale-240.png', width: 120, height: 120},
-{dest: 'Square44x44Logo.scale-100.png', width: 44, height: 44},
-{dest: 'Square44x44Logo.scale-240.png', width: 106, height: 106},
-{dest: 'Square70x70Logo.scale-100.png', width: 70, height: 70},
-{dest: 'Square71x71Logo.scale-100.png', width: 71, height: 71},
-{dest: 'Square71x71Logo.scale-240.png', width: 170, height: 170},
-{dest: 'Square150x150Logo.scale-240.png', width: 360, height: 360},
-{dest: 'Square310x310Logo.scale-100.png', width: 310, height: 310},
-{dest: 'Wide310x150Logo.scale-100.png', width: 310, height: 150},
-{dest: 'Wide310x150Logo.scale-240.png', width: 744, height: 360},
-{dest: 'SplashScreenPhone.scale-240.png', width: 1152, height: 
1920}
-];
+/** Maps MRT splashscreen image to its target project defined in 
platformImages -> 8.1|WP8.1|10
+ * This assumes we have different 
--- End diff --

Nit: incomplete comment


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
en

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89726251
  
--- Diff: cordova-js-src/confighelper.js ---
@@ -22,16 +22,17 @@
 // config.xml wrapper (non-node ConfigParser analogue)
 var config;
 function Config(xhr) {
-function loadPreferences(xhr) {
+function load(xhr, tagName) {
 var parser = new DOMParser();
 var doc = parser.parseFromString(xhr.responseText, 
"application/xml");
 
-var preferences = doc.getElementsByTagName("preference");
+var preferences = doc.getElementsByTagName(tagName);
 return Array.prototype.slice.call(preferences);
 }
 
 this.xhr = xhr;
-this.preferences = loadPreferences(this.xhr);
+this.preferences = load(this.xhr, "preference");
+this.splashScreens = load(this.xhr, "splash");
--- End diff --

Calling `load` second time here will trigger re-parsing of `config.xml` 
which, I believe, is not very efficient. Might be better to rework `load` 
method to reuse already parsed document


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89729637
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
--- End diff --

Nit: `return splash.target === targetName` is the same


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89739598
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
+} else {
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+matchesTarget = 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+return matchesTarget;
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
+
+if (matchesTarget === true) {
+extensionsUsed[target] = extensionsUsed[target] || [];
+
extensionsUsed[target].push(path.extname(img.src.toLowerCase()));
+}
+
+return matchesTarget;
+}
+
+function updateSplashExtensionInManifest(manifestFileName, 
splashScreen) {
+var manifest = AppxManifest.get(path.join(locations.root, 
manifestFileName));
+var newExtension = path.extname(splashScreen.src);
+
+if (manifest.getVisualElements().getSplashScreenExtension() !== 
newExtension) {
+events.emit('verbose', 'Set ' + manifestFileName + ' 
SplashScreen image extension to "' + newExtension + '"');
+
manifest.getVisualElements().setSplashScreenExtension(newExtension);
+manifest.write();
+}
+}
+
+var splashScreens = 
cordovaProject.projectConfig.getSplashScreens('windows');
+
+var desktopSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_DESKTOP_TARGET_NAME);
+})[0];
+
+var phoneSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_PHONE_TARGET_NAME);
+})[0];
+
+checkThatExtensionsAreNotMixed();
--- End diff --

Could you please explain, what's the purpose of this check? Please give a 
couple of examples when it would yield a warning?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89727040
  
--- Diff: template/CordovaApp.Phone.jsproj ---
@@ -79,7 +79,9 @@
 
   Designer
 
-
+
--- End diff --

Could you please explain how does this work?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #212: CB-9287 Not enough Icons and Splashscreen...

2016-11-28 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/212#discussion_r89739874
  
--- Diff: template/cordova/lib/prepare.js ---
@@ -439,6 +490,109 @@ function getUAPVersions(config) {
 };
 }
 
+/** Checks if a targetProject matches splashscreen target name */
+function splashScreenTargetProjectMatchesTargetName(targetProject, 
targetName) {
+if (targetName === SPLASH_SCREEN_DESKTOP_TARGET_NAME) {
+return targetProject === TARGET_PROJECT_81 || targetProject === 
TARGET_PROJECT_10;
+}
+
+return targetProject === TARGET_PROJECT_WP81;
+}
+
+/** Checks if the splash screen matches the target project
+ * @param targetName 'SplashScreen'|'SplashScreenPhone'
+ * @returns boolean */ 
+function splashScreenTargetIs(splash, targetName) {
+var matchesTarget, targetImg;
+
+if (splash.target) {
+// MRT syntax:
+matchesTarget = splash.target === targetName;
+} else {
+// Fall back on find by size for old non-MRT syntax:
+targetImg = findPlatformImage(splash.width, splash.height);
+matchesTarget = 
splashScreenTargetProjectMatchesTargetName(targetImg.targetProject, targetName);
+}
+
+return matchesTarget;
+}
+
+// Updates manifests to match the app splash screen image types 
(PNG/JPG/JPEG)
+function updateSplashScreenImageExtensions(cordovaProject, locations) {
+
+// Saving all extensions used for targets to verify them later
+var extensionsUsed = {};
+
+function checkThatExtensionsAreNotMixed() {
+for (var target in extensionsUsed) {
+/*jshint loopfunc: true */
+if (extensionsUsed.hasOwnProperty(target)) {
+var extensionsUsedForTarget = extensionsUsed[target];
+
+// Check that extensions are not mixed:
+if (extensionsUsedForTarget.length > 1 && 
extensionsUsedForTarget.some(function(item) {
+return item !== extensionsUsedForTarget[0];
+})) {
+events.emit('warn', '"' + target + '" splash screens 
have mixed file extensions which is not supported. Some of the images will not 
be used.');
+}
+}
+}
+}
+
+function checkTargetMatchAndUpdateUsedExtensions(img, target) {
+var matchesTarget = splashScreenTargetIs(img, target);
+
+if (matchesTarget === true) {
+extensionsUsed[target] = extensionsUsed[target] || [];
+
extensionsUsed[target].push(path.extname(img.src.toLowerCase()));
+}
+
+return matchesTarget;
+}
+
+function updateSplashExtensionInManifest(manifestFileName, 
splashScreen) {
+var manifest = AppxManifest.get(path.join(locations.root, 
manifestFileName));
+var newExtension = path.extname(splashScreen.src);
+
+if (manifest.getVisualElements().getSplashScreenExtension() !== 
newExtension) {
+events.emit('verbose', 'Set ' + manifestFileName + ' 
SplashScreen image extension to "' + newExtension + '"');
+
manifest.getVisualElements().setSplashScreenExtension(newExtension);
+manifest.write();
+}
+}
+
+var splashScreens = 
cordovaProject.projectConfig.getSplashScreens('windows');
+
+var desktopSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_DESKTOP_TARGET_NAME);
+})[0];
+
+var phoneSplashScreen = splashScreens.filter(function(img) { 
+return checkTargetMatchAndUpdateUsedExtensions(img, 
SPLASH_SCREEN_PHONE_TARGET_NAME);
+})[0];
+
+checkThatExtensionsAreNotMixed();
+
+var manifestSplashScreenMap = [{
--- End diff --

Nit: IMO dictionaly is a better candidate here than array of dictionaries 
w/ 2 keys each


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: Windows platform refactoring

2016-11-24 Thread Vladimir Kotikov (Akvelon)

Thanks, Jesse! 

-
Best regards, Vladimir
 

On 11/25/16, 10:03, "Jesse" <purplecabb...@gmail.com> wrote:

Sorry for the late reply, missed this email in the spam folder.
These changes look great, thanks!

Cheers,
  Jesse


@purplecabbage
risingj.com

On Thu, Nov 17, 2016 at 6:30 AM, Vladimir Kotikov (Akvelon) <
v-vlk...@microsoft.com> wrote:

> Hey all
>
> Just FUI, we're done some refactoring that aims to move some
> windows-specific logic (in particular, applying  changes to
> appxmanifests files) out of cordova-common into cordova-windows. I hope
> this would simplify maintenance of both cordova-common and cordova-windows
> in future
>
> Just in case if anyone is interested, here is the PR that these windows
> bits from cordova-common: 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2F=02%7C01%7Cv-vlkoti%40microsoft.com%7C4bcd445d56fa455b323e08d415013ec2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636156542464256661=3UYboUT79uLFzFHgzGUKtyw%2BYK581wdUwMAbjCRdNNI%3D=0
> cordova-lib/pull/505/files, and this PR adds them to cordova-windows:
> 
https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fcordova-windows%2Fpull%2F207=02%7C01%7Cv-vlkoti%40microsoft.com%7C4bcd445d56fa455b323e08d415013ec2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636156542464256661=qY8A8owzq1ZFKkci14luDlyBchAjPNqxIVzthidmN64%3D=0.
 Note that changes in
> windows are backward compatible, I.e. it will continue to work with
> previous versions of cordova-common, so we don't need to merge them at 
once
> and do a release ASAP
>
> I was going to merge this by tomorrow EOD, but if anyone wants to take a
> look - I would appreciate any feedback
> Thanks
>
> -
> Best regards, Vladimir
>

RE: [DISCUSS] Split cordova-lib modules into their own repos

2016-11-22 Thread Vladimir Kotikov (Akvelon)

+1 

-Original Message-
From: Filip Maj [mailto:maj@gmail.com] 
Sent: Wednesday, November 23, 2016 9:12 AM
To: dev@cordova.apache.org
Subject: Re: [DISCUSS] Split cordova-lib modules into their own repos

+1

On Tue, Nov 22, 2016 at 9:33 PM, Darryl Pogue  wrote:
> On 22 November 2016 at 17:30, Steven Gill  wrote:
>> I propose to split these modules into their own git repos. Thoughts?
>
> A giant +1 from me!
>
> I routinely run into cases where cordova-common or cordova-lib 
> problems have been fixed in master or I'm trying to test a branch, and 
> I have no way of pointing a particular project on my CI system to use 
> that branch without global npm link hackery (which doesn't even work 
> properly with npm3). Definitely excited for this!
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows issue #207: CB-12142: Move windows-specific logic from cordo...

2016-11-17 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-windows/pull/207
  
Looks good. Let's wait for tomorrow and merge it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Windows platform refactoring

2016-11-17 Thread Vladimir Kotikov (Akvelon)

Hey all

Just FUI, we're done some refactoring that aims to move some windows-specific 
logic (in particular, applying  changes to appxmanifests files) 
out of cordova-common into cordova-windows. I hope this would simplify 
maintenance of both cordova-common and cordova-windows in future

Just in case if anyone is interested, here is the PR that these windows bits 
from cordova-common: https://github.com/apache/cordova-lib/pull/505/files, and 
this PR adds them to cordova-windows: 
https://github.com/apache/cordova-windows/pull/207. Note that changes in 
windows are backward compatible, I.e. it will continue to work with previous 
versions of cordova-common, so we don't need to merge them at once and do a 
release ASAP

I was going to merge this by tomorrow EOD, but if anyone wants to take a look - 
I would appreciate any feedback
Thanks

-
Best regards, Vladimir

[GitHub] cordova-plugin-network-information issue #51: CB-11230 CB-11505 iOS: Add com...

2016-11-16 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-network-information/pull/51
  
Merging, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-plugin-network-information issue #51: CB-11230 CB-11505 iOS: Add com...

2016-11-16 Thread vladimir-kotikov

Github user vladimir-kotikov commented on the issue:

https://github.com/apache/cordova-plugin-network-information/pull/51
  
@shazron, could you take a look please? This PR just aligns curent 
implementation for `CDVReachability` with latest Apple's changes. In particular 
it removes `reachabilityForLocalWiFi` which we weren't using anyway and makes 
`reachabilityWithAddress` accept more generic `sockaddr` rather than 
`sockaddr_in`

Tested this on iOS 8 and 9 according to [this 
guide](https://developer.apple.com/library/content/documentation/NetworkingInternetWeb/Conceptual/NetworkingOverview/UnderstandingandPreparingfortheIPv6Transition/UnderstandingandPreparingfortheIPv6Transition.html#//apple_ref/doc/uid/TP40010220-CH213-SW16)
 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: CB-12142: Move windows-specific logic fro...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87808967
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+// We needn't use 'util.inherit' in this case because there aren't any 
methods on prototype in  cordova-common's PluginInfo class
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+var parentGetEditConfigs = this.getEditConfigs;
+
+this.getEditConfigs = function(platform) {
+var editConfigs = parentGetEditConfigs(platform);
+return processChanges(editConfigs);
+};
+
+this.getConfigFiles = function(platform) {
+var configFiles = parentGetConfigFiles(platform);
+return processChanges(configFiles);
+};
+
+function processChanges(changes) {
+var hasManifestChanges  = checkManifestChanges(changes);
+
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (hasManifestChanges)
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
+// Only support semver/device-target demux for 
package.appxmanifest
+// Pass through in case something downstream wants to use 
it
+if (change.target !== 'package.appxmanifest') {
+changes.push(change);
+return;
+}
+
+// No semver/device-target for this config-file, pass it 
through
+if (!(hasChangeVersion(change) || 
hasChangeTarget(change))) {
+// New windows template separate manifest files for 
Windows10, Windows8.1 and WP8.1
+changes = 
changes.concat(demuxChangeWithSubsts(change));
+return;
+}
+
+var targetDeviceSet = getDeviceTargetSetForChange(change);
+changes = replaceChangeTarget(targetDeviceSet, change, 
changes);
+});
+}
+return changes;
+}
+
+function checkManifestChanges(changes) {
--- End diff --

You're calling this only once - is there a real need to wrap this into 
function?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: CB-12142: Move windows-specific logic fro...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87807973
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+// We needn't use 'util.inherit' in this case because there aren't any 
methods on prototype in  cordova-common's PluginInfo class
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+var parentGetEditConfigs = this.getEditConfigs;
+
+this.getEditConfigs = function(platform) {
+var editConfigs = parentGetEditConfigs(platform);
+return processChanges(editConfigs);
+};
+
+this.getConfigFiles = function(platform) {
+var configFiles = parentGetConfigFiles(platform);
+return processChanges(configFiles);
+};
+
+function processChanges(changes) {
--- End diff --

I'd suggest to move this out of `PluginInfo` - this way it'd be possible to 
use this function in tests (using `rewire`)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: CB-12142: Move windows-specific logic fro...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87810709
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+// We needn't use 'util.inherit' in this case because there aren't any 
methods on prototype in  cordova-common's PluginInfo class
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+var parentGetEditConfigs = this.getEditConfigs;
+
+this.getEditConfigs = function(platform) {
+var editConfigs = parentGetEditConfigs(platform);
+return processChanges(editConfigs);
+};
+
+this.getConfigFiles = function(platform) {
+var configFiles = parentGetConfigFiles(platform);
+return processChanges(configFiles);
+};
+
+function processChanges(changes) {
+var hasManifestChanges  = checkManifestChanges(changes);
+
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (hasManifestChanges)
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
+// Only support semver/device-target demux for 
package.appxmanifest
+// Pass through in case something downstream wants to use 
it
+if (change.target !== 'package.appxmanifest') {
+changes.push(change);
+return;
+}
+
+// No semver/device-target for this config-file, pass it 
through
+if (!(hasChangeVersion(change) || 
hasChangeTarget(change))) {
+// New windows template separate manifest files for 
Windows10, Windows8.1 and WP8.1
+changes = 
changes.concat(demuxChangeWithSubsts(change));
+return;
+}
+
+var targetDeviceSet = getDeviceTargetSetForChange(change);
+changes = replaceChangeTarget(targetDeviceSet, change, 
changes);
+});
+}
+return changes;
+}
+
+function checkManifestChanges(changes) {
+return changes.some(function(change) {
+return change.target === 'package.appxmanifest';
+});
+}
+function hasChangeVersion(change) {
+return (typeof change.versions !== 'undefined');
+}
+
+function hasChangeTarget(change) {
+return (typeof change.deviceTarget !== 'undefined');
+}
+
+function demuxChangeWithSubsts(change) {
+return SUBSTS.map(function(subst) {
+ return createChangeWithNewTarget(change, subst);
+});
+}
+
+function createChangeWithNewTarget(change, substTarget) {
+var changeWithNewTarget = {};
+Object.keys(change).forEach(function(changeKey) {
+changeWithNewTarget[changeKey] = change[changeKey];
+});
+changeWithNewTarget.target = substTarget;
+return changeWithNewTarget;
+}
+
+function getDeviceTargetSetForChange(change) {
+var targetDeviceSet = hasChangeTarget(change) ? 
change.deviceTarget : 'all';
+if (TARGETS.indexOf(targetDeviceSet) === -1) {
+// target-device couldn't be resolved, fix it up here to a 
valid value
+targetDeviceSet = 'all';
+}
+return targetDeviceSet;
+}
+
+// This is a local function that creates the new replacement 
representing the
+// mutation

[GitHub] cordova-windows pull request #207: CB-12142: Move windows-specific logic fro...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87809002
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+// We needn't use 'util.inherit' in this case because there aren't any 
methods on prototype in  cordova-common's PluginInfo class
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+var parentGetEditConfigs = this.getEditConfigs;
+
+this.getEditConfigs = function(platform) {
+var editConfigs = parentGetEditConfigs(platform);
+return processChanges(editConfigs);
+};
+
+this.getConfigFiles = function(platform) {
+var configFiles = parentGetConfigFiles(platform);
+return processChanges(configFiles);
+};
+
+function processChanges(changes) {
+var hasManifestChanges  = checkManifestChanges(changes);
+
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (hasManifestChanges)
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
+// Only support semver/device-target demux for 
package.appxmanifest
+// Pass through in case something downstream wants to use 
it
+if (change.target !== 'package.appxmanifest') {
+changes.push(change);
+return;
+}
+
+// No semver/device-target for this config-file, pass it 
through
+if (!(hasChangeVersion(change) || 
hasChangeTarget(change))) {
+// New windows template separate manifest files for 
Windows10, Windows8.1 and WP8.1
+changes = 
changes.concat(demuxChangeWithSubsts(change));
+return;
+}
+
+var targetDeviceSet = getDeviceTargetSetForChange(change);
+changes = replaceChangeTarget(targetDeviceSet, change, 
changes);
+});
+}
+return changes;
+}
+
+function checkManifestChanges(changes) {
+return changes.some(function(change) {
+return change.target === 'package.appxmanifest';
+});
+}
+function hasChangeVersion(change) {
--- End diff --

Ditto


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: CB-12142: Move windows-specific logic fro...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87810296
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+// We needn't use 'util.inherit' in this case because there aren't any 
methods on prototype in  cordova-common's PluginInfo class
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+var parentGetEditConfigs = this.getEditConfigs;
+
+this.getEditConfigs = function(platform) {
+var editConfigs = parentGetEditConfigs(platform);
+return processChanges(editConfigs);
+};
+
+this.getConfigFiles = function(platform) {
+var configFiles = parentGetConfigFiles(platform);
+return processChanges(configFiles);
+};
+
+function processChanges(changes) {
+var hasManifestChanges  = checkManifestChanges(changes);
+
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (hasManifestChanges)
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
+// Only support semver/device-target demux for 
package.appxmanifest
+// Pass through in case something downstream wants to use 
it
+if (change.target !== 'package.appxmanifest') {
+changes.push(change);
+return;
+}
+
+// No semver/device-target for this config-file, pass it 
through
+if (!(hasChangeVersion(change) || 
hasChangeTarget(change))) {
+// New windows template separate manifest files for 
Windows10, Windows8.1 and WP8.1
+changes = 
changes.concat(demuxChangeWithSubsts(change));
+return;
+}
+
+var targetDeviceSet = getDeviceTargetSetForChange(change);
+changes = replaceChangeTarget(targetDeviceSet, change, 
changes);
--- End diff --

Consider renaming `replaceChangeTarget` to something more appropriate as it 
doesn't really replace anything in the original array


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: CB-12142: Move windows-specific logic fro...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87809027
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,150 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+var SUBSTS = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+var TARGETS = ['windows', 'phone', 'all'];
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+// We needn't use 'util.inherit' in this case because there aren't any 
methods on prototype in  cordova-common's PluginInfo class
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+var parentGetEditConfigs = this.getEditConfigs;
+
+this.getEditConfigs = function(platform) {
+var editConfigs = parentGetEditConfigs(platform);
+return processChanges(editConfigs);
+};
+
+this.getConfigFiles = function(platform) {
+var configFiles = parentGetConfigFiles(platform);
+return processChanges(configFiles);
+};
+
+function processChanges(changes) {
+var hasManifestChanges  = checkManifestChanges(changes);
+
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (hasManifestChanges)
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
+// Only support semver/device-target demux for 
package.appxmanifest
+// Pass through in case something downstream wants to use 
it
+if (change.target !== 'package.appxmanifest') {
+changes.push(change);
+return;
+}
+
+// No semver/device-target for this config-file, pass it 
through
+if (!(hasChangeVersion(change) || 
hasChangeTarget(change))) {
+// New windows template separate manifest files for 
Windows10, Windows8.1 and WP8.1
+changes = 
changes.concat(demuxChangeWithSubsts(change));
+return;
+}
+
+var targetDeviceSet = getDeviceTargetSetForChange(change);
+changes = replaceChangeTarget(targetDeviceSet, change, 
changes);
+});
+}
+return changes;
+}
+
+function checkManifestChanges(changes) {
+return changes.some(function(change) {
+return change.target === 'package.appxmanifest';
+});
+}
+function hasChangeVersion(change) {
+return (typeof change.versions !== 'undefined');
+}
+
+function hasChangeTarget(change) {
--- End diff --

Same here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: Move windows-specific logic from cordova-...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87751813
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+this.getConfigFiles = function(platform) {
+var changes = parentGetConfigFiles(platform);
+var editChanges = this.getEditConfigs(platform);
+if (editChanges && editChanges.length !== 0) {
+changes.push(editChanges);
+}
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (changes.some(function(change) {
--- End diff --

Nit: could you please invert `if` condition here? Also it might be more 
convenient to assign this condition to intermediate variable, something like 
`var hasManifestChanges = changes.some(function(change) { return change.target 
=== 'package.appxmanifest' })`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: Move windows-specific logic from cordova-...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87752512
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+this.getConfigFiles = function(platform) {
+var changes = parentGetConfigFiles(platform);
+var editChanges = this.getEditConfigs(platform);
+if (editChanges && editChanges.length !== 0) {
+changes.push(editChanges);
+}
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (changes.some(function(change) {
+return change.target === 'package.appxmanifest';
+}))
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
+// Only support semver/device-target demux for 
package.appxmanifest
+// Pass through in case something downstream wants to use 
it
+if (change.target !== 'package.appxmanifest') {
+changes.push(change);
+return;
+}
+
+var hasVersion = (typeof change.versions !== 'undefined');
+var hasTargets = (typeof change.deviceTarget !== 
'undefined');
+
+// No semver/device-target for this config-file, pass it 
through
+if (!(hasVersion || hasTargets)) {
+// New windows template separate manifest files for 
Windows10, Windows8.1 and WP8.1
+var substs = ['package.phone.appxmanifest', 
'package.windows.appxmanifest', 'package.windows10.appxmanifest'];
+changes = changes.concat(substs.map(function(subst) {
+return Object.assign({}, change, {target: subst});
+}));
+return changes;
--- End diff --

Nit: you can just `return` here - result of this function is not used 
anywhere


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: Move windows-specific logic from cordova-...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87752779
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+CommonPluginInfo.apply(this, arguments);
--- End diff --

Pls add a comment, clarifying why you're not using `util.inherit` to build 
a children class


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: Move windows-specific logic from cordova-...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87751321
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
--- End diff --

Rather than saving this into a variable i'd propose to call 
`ThisClass.super_.prototype.generate_plugin_config_munge` method directly


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-windows pull request #207: Move windows-specific logic from cordova-...

2016-11-14 Thread vladimir-kotikov

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-windows/pull/207#discussion_r87752877
  
--- Diff: template/cordova/lib/PluginInfo.js ---
@@ -0,0 +1,109 @@
+var semver = require('semver');
+var CommonPluginInfo = require('cordova-common').PluginInfo;
+
+var MANIFESTS = {
+'windows': {
+'8.1.0': 'package.windows.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'phone': {
+'8.1.0': 'package.phone.appxmanifest',
+'10.0.0': 'package.windows10.appxmanifest'
+},
+'all': {
+'8.1.0': ['package.windows.appxmanifest', 
'package.phone.appxmanifest'],
+'10.0.0': 'package.windows10.appxmanifest'
+}
+};
+
+/*
+A class for holidng the information currently stored in plugin.xml
+It's inherited from cordova-common's PluginInfo class
+In addition it overrides getConfigFiles method to use windows-specific 
logic
+ */
+function PluginInfo(dirname) {
+CommonPluginInfo.apply(this, arguments);
+var parentGetConfigFiles = this.getConfigFiles;
+this.getConfigFiles = function(platform) {
+var changes = parentGetConfigFiles(platform);
+var editChanges = this.getEditConfigs(platform);
+if (editChanges && editChanges.length !== 0) {
+changes.push(editChanges);
+}
+// Demux 'package.appxmanifest' into relevant platform-specific 
appx manifests.
+// Only spend the cycles if there are version-specific plugin 
settings
+if (changes.some(function(change) {
+return change.target === 'package.appxmanifest';
+}))
+{
+var oldChanges = changes;
+changes = [];
+
+oldChanges.forEach(function(change, changeIndex) {
--- End diff --

A general comment - can you please divide this function into smaller 
pieces, so it'd be easier to review and test?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 1460 matches

Mail list logo