Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-24750] Only configure target platform #8

Merged
merged 3 commits into from May 31, 2017

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented May 30, 2017

  • Only configure the platform we intent to build, for faster build times
TEST CASE
  • Should not configure windows or ios
appc run -p android --build-only
NOTES: Unfortunately cli.argv.platform is not defined at this point in the process

lib/titanium.js Outdated
@@ -52,7 +52,8 @@ exports.commonOptions = function (logger, config) {
};

exports.platformOptions = function (logger, config, cli, commandName, finished) {
var result = {};
var result = {},
targetPlatform = cli.argv['platform'] || cli.argv['p'];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to reference these using brackets.

lib/titanium.js Outdated
@@ -75,6 +76,12 @@ exports.platformOptions = function (logger, config, cli, commandName, finished)
// for each platform, fetch their specific flags/options
async.parallel(manifest.platforms.map(function (platform) {
return function (callback) {

// only configure target platform
if (targetPlatform && platform != targetPlatform) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use strict equality: !==.

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the help command. If you do ti build -p ios --help, it does not display the complete help for the build command. You need to check if -h or --help was set. I'm not sure off the top of my head how this is treated. Maybe commandName is set to help? Maybe there's a boolean cli.argv.help? You'll have to poke around.

@garymathews
Copy link
Contributor Author

  • Fixed help flag, cli.argv.help covers both --help and -h
  • Used strict equality

@cb1kenobi
Copy link
Contributor

Looks good! APPROVED!

@cb1kenobi cb1kenobi merged commit 47ad3e0 into tidev:master May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants