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

[BUGFIX] Fix custom blueprint options for destroy command #4104

Merged
merged 3 commits into from May 22, 2015

Conversation

trabus
Copy link
Contributor

@trabus trabus commented May 18, 2015

This PR fixes #4099 by adding custom blueprint options to the destroy command (previously was only added to the generate command). The ember destroy component foo-bar -p command was failing because of the addition of an option that the __path__ token depended upon.

This PR also fixes all of the internal destroy acceptance tests by asserting that the destroy command returns an object instead of an errorCode.

// merge in blueprint availableOptions
var blueprint;
var debug = require('debug')('ember-cli/commands/destroy');
try{
Copy link
Contributor

Choose a reason for hiding this comment

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

whitespace

@trabus
Copy link
Contributor Author

trabus commented May 19, 2015

So now that I'm rethrowing the error, it actually exposed that the cli-test.js unit test was swallowing an error previously. The cli stub was missing a method and throwing an error.

@stefanpenner
Copy link
Contributor

So now that I'm rethrowing the error, it actually exposed that the cli-test.js unit test was swallowing an error previously. The cli stub was missing a method and throwing an error.

oh wow, good catch!

@trabus trabus force-pushed the destroy-fix branch 2 times, most recently from ecc2eef to d5d30ea Compare May 19, 2015 18:11
@trabus
Copy link
Contributor Author

trabus commented May 19, 2015

This file-info unit test failure started after rebasing from master https://travis-ci.org/ember-cli/ember-cli/jobs/63214480#L1960

@stefanpenner I'll rebase after #4114 is merged

@stefanpenner
Copy link
Contributor

@stefanpenner I'll rebase after #4114 is merged

merged

@stefanpenner
Copy link
Contributor

@trabus can you rebase, i fixed some windows issue.

be sure to review: https://ci.appveyor.com/project/embercli/ember-cli/history rather then the widget inlined in GH, as the widget for some reason points to @jayphelps's appveyor...

@jayphelps
Copy link
Member

I'm gonna contact their support and see if I can get it to stop pointing at mine some how...I imagine it was some sort of autodetection that picked it up when I was added ass a collab'r or something.

@trabus
Copy link
Contributor Author

trabus commented May 22, 2015

I don't understand how coveralls is saying this decreased coverage, when I just added a test for the thing it was complaining about. It was previously decreased by -0.06%, but adding a test for what it was complaining about decreased it by -0.24%.

@stefanpenner
Copy link
Contributor

Maybe a rounding error?

@stefanpenner
Copy link
Contributor

Wanna maybe extract the code in the catch block to a shared function?

@trabus
Copy link
Contributor Author

trabus commented May 22, 2015

I'll try that later today.

stefanpenner added a commit that referenced this pull request May 22, 2015
[BUGFIX] Fix custom blueprint options for destroy command
@stefanpenner stefanpenner merged commit d22a1ab into ember-cli:master May 22, 2015
@stefanpenner
Copy link
Contributor

I'll try that later today.

#4148

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.

ember [generate|delete] component fails
4 participants