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

Moved reverse() to after filter() so we can keep consistent order #3244

Merged
merged 1 commit into from
Feb 11, 2015

Conversation

nathanpalmer
Copy link
Contributor

I have a project that has multiple "overrides" for blueprints. However the paths for the main blueprint and the test blueprint were coming up in an inconsistent order. That means I would get the main blueprint for one project and the test blueprint for another.

To help illustrate the problem I have this

  1. ember-cli
  2. ember-coffeescript
  3. in-repo addon

When I run this command

ember g adapter users --dry-run

It chooses the adapter blueprint from in-repo addon and the adapter-test blueprint from ember-coffeescript. Even-though both blueprints exist in the in-repo addon. When I traced it down it seemed to work everytime as long as we reverse after the filter.

@trabus
Copy link
Contributor

trabus commented Feb 10, 2015

Do you think it would be much work to add a test for this? Would probably be a good thing to check for regression.

@stefanpenner
Copy link
Contributor

@trabus is correct, this detail must be tested.

@nathanpalmer
Copy link
Contributor Author

@trabus @stefanpenner I added a test that shows that it fails when we call this method twice. This is what happens when we get the lookupPaths for the main blueprint and then a second time for the test blueprint

@stefanpenner
Copy link
Contributor

@nathanpalmer any luck on getting the tests passing though?

@nathanpalmer
Copy link
Contributor Author

@stefanpenner The test I added and the test I modified are passing just fine.

screenshot 2015-02-10 17 11 25

The master build however wasn't passing when I forked. It seems only related to the iojs build starting with this commit and this build.

@trabus
Copy link
Contributor

trabus commented Feb 10, 2015

It's actually been failing since this was merged.

As far as the tests go, lgtm 👍

@trabus
Copy link
Contributor

trabus commented Feb 10, 2015

If you rebase after #3250 gets merged in, we can be sure.

@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2015

#3250 is merged, can you rebase?

@nathanpalmer
Copy link
Contributor Author

Sure, I'll rebase when I get home.

rwjblue added a commit that referenced this pull request Feb 11, 2015
Moved reverse() to after filter() so we can keep consistent order
@rwjblue rwjblue merged commit c8934ab into ember-cli:master Feb 11, 2015
@rwjblue
Copy link
Member

rwjblue commented Feb 11, 2015

Thanks!

@nathanpalmer nathanpalmer deleted the addon-order branch February 11, 2015 02:25
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

4 participants