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

Added an option to --skip-cleanup #18

Merged
merged 1 commit into from
Apr 13, 2015

Conversation

jrjohnson
Copy link
Contributor

When running in a CI environment like travis the cleanup is just extra
time wasted. —skip-cleanup will prevent the reset from running.

I only added this to the try command, if you want I’m happy to add it to testall as well, but I'm not sure if that has the same use case.

I did a thing with a fake promise in the try task that might be the wrong approach, but I'm not super familiar with writing commands and I wasn't sure what needed to be returned from where. Alternatively I could skip the promise and just exit, but then the output would have to be managed in two places.
All that is to say - let me know if you want me to do this another way.

var promise;
if(commandOptions.skipCleanup){
//create a fake promise for consistency
promise = new RSVP.Promise(function(resolve) {
Copy link
Member

Choose a reason for hiding this comment

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

Can just be promise = RSVP.Promise.resolve();.

@jrjohnson
Copy link
Contributor Author

Thanks @rwjblue I knew there was a better way to do that, but my google-foo failed me.

@kategengler
Copy link
Member

Thanks @jrjohnson.

I think something we should warn about: If you use this option and, without cleaning up, build and deploy as the result of a passing test suite, it will build with the last set of dependencies ember try was run with.

I think it would make sense for testall to also have this option. I expect testall to be the command more frequently used in CI.

@jrjohnson
Copy link
Contributor Author

@kategengler - where should I put the warning? In the README, as output after the command is run, or in the command options itself?

@kategengler
Copy link
Member

The README would be good, thanks.

On Thu, Apr 9, 2015 at 4:01 AM, Jonathan Johnson [email protected]
wrote:

@kategengler https://github.com/kategengler - where should I put the
warning? In the README, as output after the command is run, or in the
command options itself?


Reply to this email directly or view it on GitHub
#18 (comment).

@jrjohnson
Copy link
Contributor Author

Updated the README with @kategengler's warning, added --skip-cleanup to testall, and fixed my original code to not pass that option on to ember. Let me know if you want me to squash all these commits, I left them separate so they were easier to review.

@kategengler
Copy link
Member

Looks good, it'd be great if you could squash.

try and try:testall commands now have an option to --skip-cleanup which prevents bower from being restored to its default state.  This is usefull in CI environments where the code is being thrown out and not deployed.
@jrjohnson
Copy link
Contributor Author

ok, all set.

kategengler added a commit that referenced this pull request Apr 13, 2015
Added an option to --skip-cleanup
@kategengler kategengler merged commit 35f28ae into ember-cli:master Apr 13, 2015
@jrjohnson jrjohnson deleted the skipcleanupoption branch April 13, 2015 04:50
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.

3 participants