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-11121: CLI: Create wrapper around existing android builder.py #3085

Merged
merged 3 commits into from Oct 2, 2012

Conversation

ayeung
Copy link
Contributor

@ayeung ayeung commented Sep 29, 2012

@ghost ghost assigned nebrius Oct 1, 2012
logger.info(__('Compiling "%s" build', cli.argv['deploy-type']));


ti.legacy.constructLegacyCommand(logger, cli, tiapp,cli.argv.platform ,cmd, emulatorCmd);
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around commas are inconsistent

@nebrius
Copy link
Contributor

nebrius commented Oct 1, 2012

If any of the python scripts crash currently, that crash goes undetected and the build process continues on. A crash should be detected (return code probably) and the build should fail then and there.


dump(cli.argv);
console.log('Forking correct SDK command: ' + (cmd.join(' ')).cyan + '\n');
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this log be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say leave it, but comment it out.

@nebrius
Copy link
Contributor

nebrius commented Oct 1, 2012

When building for the playstore, if the output directory doesn't exist, it should be automatically created

@nebrius
Copy link
Contributor

nebrius commented Oct 1, 2012

If android build fails, we should not print that build was successful

@nebrius
Copy link
Contributor

nebrius commented Oct 1, 2012

Paths should be converted to absolute using path.resolve (specifically --output-dir)

@nebrius
Copy link
Contributor

nebrius commented Oct 1, 2012

Need to add logic for the -b (build only) flag

@cb1kenobi
Copy link
Contributor

Regarding the paths should be converted to absolute paths, use appc.fs.resolvePath(), not path.resolve(). path.resolve() does not resolve paths that start with "~".

@cb1kenobi
Copy link
Contributor

I've changed up how a build reports if it was successful or not. If the Android build fails, simply call the finished() callback with anything truthy (i.e. the "err"). The message will print correctly once my I do a PR for my stuff.

- Resolve paths correctly
- Check exit code on spawn process and error accordingly
- Commented out debugging code
- Support build-only option
@ayeung
Copy link
Contributor Author

ayeung commented Oct 2, 2012

Code updated. Ready for review again.

@nebrius
Copy link
Contributor

nebrius commented Oct 2, 2012

Code reviewed and tested. Request accepted

nebrius pushed a commit that referenced this pull request Oct 2, 2012
TIMOB-11121: CLI: Create wrapper around existing android builder.py
@nebrius nebrius merged commit 7133e70 into tidev:master Oct 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants