Skip to content

[BUGFIX beta] Fix ember-routing-inherits-parent-model for nestable routes.#5610

Merged
rwjblue merged 1 commit intoemberjs:masterfrom
ef4:always-inherit
Sep 20, 2014
Merged

[BUGFIX beta] Fix ember-routing-inherits-parent-model for nestable routes.#5610
rwjblue merged 1 commit intoemberjs:masterfrom
ef4:always-inherit

Conversation

@ef4
Copy link
Contributor

@ef4 ef4 commented Sep 19, 2014

When the original ember-routing-inherits-parent-model feature was accepted, it was decided that routes could inherit their parent's model but resources couldn't.

But now the distinction between those two is smaller than ever, and the existing check is actually wrong, since it excludes routes with children.

I'm proposing we scrap this legacy distinction and just let any route inherit.

Copy link
Member

Choose a reason for hiding this comment

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

This test should only be run if the feature is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right! Thanks.

@stefanpenner
Copy link
Member

@wycats r?

@rwjblue
Copy link
Member

rwjblue commented Sep 19, 2014

👍 from me

@rwjblue
Copy link
Member

rwjblue commented Sep 19, 2014

@ef4 - All features need to add an entry in features.json (value should be null) and FEATURES.md.

@rwjblue
Copy link
Member

rwjblue commented Sep 19, 2014

Now that I think about this more, this kinda feels like a bugfix not really a "new feature".

Any objections to just using [BUGFIX beta]?

@ef4
Copy link
Contributor Author

ef4 commented Sep 19, 2014

I added it to the feature files.

BUGFIX is fine too. One could argue that this should have been part of the nested-route change. I don't know of any cases where this will break people's apps unless they're way off the happy path.

@rwjblue
Copy link
Member

rwjblue commented Sep 19, 2014

Yes, exactly my thoughts. If I had thought of it, I would have change this when I did the nested route stuff. I'll flag for review in the core team meeting today to decide on BUGFIX or FEATURE.

@rwjblue
Copy link
Member

rwjblue commented Sep 19, 2014

Just discussed this in the core team meeting. Definitely a BUGFIX.

@ef4 - Can you remove the feature flagging, and update the commit prefix with [BUGFIX beta]? Also, sorry for the mixed signals here..

@rwjblue rwjblue changed the title [FEATURE ember-routing-always-inherit-parent-model] Fix ember-routing-inherits-parent-model for nestable routes. Sep 20, 2014
rwjblue added a commit that referenced this pull request Sep 20, 2014
Fix ember-routing-inherits-parent-model for nestable routes.
@rwjblue rwjblue merged commit dc1042d into emberjs:master Sep 20, 2014
@rwjblue rwjblue changed the title Fix ember-routing-inherits-parent-model for nestable routes. [BUGFIX beta] Fix ember-routing-inherits-parent-model for nestable routes. Sep 20, 2014
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.

4 participants