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

Fix: "ti clean" does not scan SDK hooks #7628

Merged
merged 1 commit into from Jan 14, 2016
Merged

Conversation

infosia
Copy link
Contributor

@infosia infosia commented Jan 12, 2016

Fix for TIMOB-20085

Currently there's no chance for Titanium SDK to load "hooks" while ti clean even when you subscribe clean.post event. It is required from Windows SDK to load hooks before ti clean because it uses special directory for Visual Studio.

This PR basically gives platform-SDK a chance to load hooks before ti clean, via scanHooks function in _clean.js. platform-SDK that needs clean-hook should scan hooks in cli/commands/_clean.js, and then subscribe clean.pre, clean.post and so on.

_cli/commands/clean.js

var path = require('path');

exports.scanHooks = function(logger, config, cli) {
    cli.scanHooks(path.join(__dirname, '..', 'hooks'));
}

cli/hooks/clean.js

 exports.init = function (logger, config, cli) {
    cli.on('clean.post', {
        post: function (builder, finished) {
... // platform-specific clean-up
            finished();
        }
    });
};

See discussion in https://github.com/appcelerator/titanium_mobile_windows/pull/505 for more details.

@@ -89,6 +89,11 @@ exports.run = function (logger, config, cli) {
if (cli.argv.platforms) {
async.series(cli.argv.platforms.map(function (platform) {
return function (next) {
// scan platform SDK specific clean hooks
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need to look for a platform-specific cli/commands/_clean.js, load that and call scanHooks on it? Shouldn't we just call:

cli.scanHooks(path.join(__dirname, '..', '..', platform, 'cli', 'hooks'));

Basically just say load all hooks from the platform specific cli hooks directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just wanted to make it flexible for platform SDK to choose hook directory but I might be thinking too much.

@infosia infosia assigned infosia and unassigned sgtcoolguy Jan 14, 2016
sgtcoolguy added a commit that referenced this pull request Jan 14, 2016
Fix: "ti clean" does not scan SDK hooks
@sgtcoolguy sgtcoolguy merged commit edc31cb into tidev:master Jan 14, 2016
@infosia infosia deleted the TIMOB-20085 branch December 1, 2016 01:29
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

2 participants