You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by sgrebnov <gi...@git.apache.org> on 2014/06/20 15:20:32 UTC
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
GitHub user sgrebnov opened a pull request:
https://github.com/apache/cordova-cli/pull/181
CB-6976 Add support for Windows Universal apps (Windows 8.1 and WP 8.1)
https://issues.apache.org/jira/browse/CB-6976
* Allow using new windows alias to work with old windows8 platform
* Allow using old windows8 alias to work with new windows universal apps
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-cli win81
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-cli/pull/181.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 #181
----
commit 4de816dd5f9454f35ea75b8667fe55c4b8868483
Author: sgrebnov <v-...@microsoft.com>
Date: 2014-06-20T13:17:45Z
CB-6976 Add support for Windows Universal apps (Windows 8.1 and WP 8.1)
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/181#discussion_r14055705
--- Diff: src/cli.js ---
@@ -165,6 +165,20 @@ function cli(inputArgs) {
throw new CordovaError(msg)
}
+ // CB-6976 Windows Universal Apps. Allow mixing windows and windows8 aliases
+ opts.platforms = opts.platforms.map(function(platform) {
+ // allow using old windows8 alias for new unified windows platform
+ if (platform == 'windows8' && require('fs').existsSync('platforms/windows')) {
--- End diff --
Will be updated, thx @kamrik!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/181#discussion_r14029489
--- Diff: src/cli.js ---
@@ -165,6 +165,20 @@ function cli(inputArgs) {
throw new CordovaError(msg)
}
+ // CB-6976 Windows Universal Apps. Allow mixing windows and windows8 aliases
+ opts.platforms = opts.platforms.map(function(platform) {
+ // allow using old windows8 alias for new unified windows platform
+ if (platform == 'windows8' && require('fs').existsSync('platforms/windows')) {
+ return 'windows';
+ }
+ // allow using new windows alias for old windows8 platform
+ if (platform == 'windows' && !require('fs').existsSync('platforms/windows')
+ && require('fs').existsSync('platforms/windows8')) {
+ return 'windows8';
--- End diff --
Looks like this makes it impossible to use whatever is in platforms/windows.
Both widows and windows8 platforms passed to the cli will refer to platforms/windows8
Is this the intent?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/181#discussion_r14029634
--- Diff: src/cli.js ---
@@ -165,6 +165,20 @@ function cli(inputArgs) {
throw new CordovaError(msg)
}
+ // CB-6976 Windows Universal Apps. Allow mixing windows and windows8 aliases
+ opts.platforms = opts.platforms.map(function(platform) {
+ // allow using old windows8 alias for new unified windows platform
--- End diff --
Do we really want to allow that? Sounds like it's better to warn here and ask to stop using the windows8 alias. If they already know about the unified platform and even added it, why would they use the windows8 alias? And if they do out of habit, why would we encourage that?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by kamrik <gi...@git.apache.org>.
Github user kamrik commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/181#discussion_r14029357
--- Diff: src/cli.js ---
@@ -165,6 +165,20 @@ function cli(inputArgs) {
throw new CordovaError(msg)
}
+ // CB-6976 Windows Universal Apps. Allow mixing windows and windows8 aliases
+ opts.platforms = opts.platforms.map(function(platform) {
+ // allow using old windows8 alias for new unified windows platform
+ if (platform == 'windows8' && require('fs').existsSync('platforms/windows')) {
--- End diff --
please, require('fs') at the top of the 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/181#discussion_r14030140
--- Diff: src/cli.js ---
@@ -165,6 +165,20 @@ function cli(inputArgs) {
throw new CordovaError(msg)
}
+ // CB-6976 Windows Universal Apps. Allow mixing windows and windows8 aliases
+ opts.platforms = opts.platforms.map(function(platform) {
+ // allow using old windows8 alias for new unified windows platform
--- End diff --
That is fine, and already discussed and decided.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on the pull request:
https://github.com/apache/cordova-cli/pull/181#issuecomment-46787820
@kamrik, @purplecabbage thx for review and notes
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-cli/pull/181
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-cli pull request: CB-6976 Add support for Windows Universa...
Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/181#discussion_r14030680
--- Diff: src/cli.js ---
@@ -165,6 +165,20 @@ function cli(inputArgs) {
throw new CordovaError(msg)
}
+ // CB-6976 Windows Universal Apps. Allow mixing windows and windows8 aliases
+ opts.platforms = opts.platforms.map(function(platform) {
+ // allow using old windows8 alias for new unified windows platform
+ if (platform == 'windows8' && require('fs').existsSync('platforms/windows')) {
+ return 'windows';
+ }
+ // allow using new windows alias for old windows8 platform
+ if (platform == 'windows' && !require('fs').existsSync('platforms/windows')
+ && require('fs').existsSync('platforms/windows8')) {
+ return 'windows8';
--- End diff --
Discussion is here.
http://markmail.org/message/v5pul7xqzp5ndnto
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---