ember-fastboot / ember-cli-head

Ember Cli Addon for Adding Content to HTML Head
MIT License
98 stars 34 forks source link

Make addon more idiomatic at the expense of making it 2.10+ & fastboot 1.0rc+ #32

Closed cibernox closed 7 years ago

cibernox commented 7 years ago

Added a note on the readme explaining that.

kratiahuja commented 7 years ago

Might as well move app/instance-initializer/fastboot/head.js to fastboot/instance-initializer/head.js since this is a major version bump ?

cibernox commented 7 years ago

@kratiahuja I don't follow. That is precisely what I did.

kratiahuja commented 7 years ago

Super sorry :( those changes showed up very small in the diff so I got confused. This LGTM.

simonihmig commented 7 years ago

@cibernox can you please explain the rationale for these changes? A bit of work went into #21 to not make any breaking changes, so it now runs with beta and rc version of FastBoot as well as those older Ember versions.

I understand the required code became quite a bit confusing when looking at it, but as this addon is probably used by most FastBoot apps, I think a higher degree of compatibility would make sense. At least until we feel safe to drop support for older version, say when FastBoot 1.0 stable has been released and most other addons have been migrated (still a long way, see https://github.com/ember-fastboot/ember-cli-fastboot/issues/387). Then the changes here make absolutely sense to me!

cibernox commented 7 years ago

I just asked @ronco if leaving 0.2.2 as a bridge version and release a simpler and leaner 0.3 version that is 1.0+ only was ok. He seemed to be ok, people using fastboot beta can or non-glimmer ember can continue to use the old version.

This PR also makes #30 irrelevant, so it's like 3 PRs in one.

ronco commented 7 years ago

LGTM. @simonihmig are you ok with dropping pre-1.0 Fastboot support with a 0.3 version bump?

simonihmig commented 7 years ago

@ronco Sure, if it's ok for you, so it is for me! :)

rwjblue commented 7 years ago

agreed, I like this direction