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

[fixes #2723 #2813] opt-out flag for slash being the namespace separator. #3537

Conversation

stefanpenner
Copy link
Member

(cc @kselden)

@stefanpenner
Copy link
Member Author

i suppose i should use feature instead of ENV flag... thoughts?

@hjdivad
Copy link
Member

hjdivad commented Oct 7, 2013

@stefanpenner I presume feature flags are things that would one day go away once the feature makes it into a release. This seems different from runtime config flags.

@wagenet
Copy link
Member

wagenet commented Oct 8, 2013

@stefanpenner I recall @wycats having doubts about long running flags like this. Is this an exceptional case?

@stefanpenner
Copy link
Member Author

@wagenet i'm unsure what the other option is, unless we want to break backwards compat, or have more hacks. I am very open to suggestions.

The current namespace solution is basically broken, but people may depend on it. As it is broken, and we have a successor planned, we would like to eventually phase it out. Most of the complexity is within the resolver, but that render helper hack leaks complexity all over the place.

@wagenet
Copy link
Member

wagenet commented Oct 11, 2013

@stefanpenner We should discuss this at the core team meeting tomorrow.

@stefanpenner
Copy link
Member Author

lets add this to this upcoming meeting, as I was sick and missed the last one.

@ghost ghost assigned stefanpenner Nov 9, 2013
@stefanpenner
Copy link
Member Author

I should likely wrap this up in some more DefaultResolver + normalization improvements.

@wagenet
Copy link
Member

wagenet commented Dec 8, 2013

@stefanpenner status?

@stefanpenner
Copy link
Member Author

@wagenet you had concerns about long running flags, what is your current thought on that.

This slash as namespace is actually quite broken. I would love to know how many people use it, or rely on it.

@stefanpenner
Copy link
Member Author

note to self, should try to extract this into the resolver.

@rwjblue
Copy link
Member

rwjblue commented Dec 22, 2013

I believe that this is likely a good case (see #3936 for another example) for feature flags in the stable build. By flagging this change, and leaving the value null in features.json consumers could opt-out of the slash-as-namespace issue all together.

@limptwiglet
Copy link

Just wondering if there has been any more thoughts on this as its causing problems for myself.

@wagenet
Copy link
Member

wagenet commented Apr 12, 2014

@stefanpnner status? If this isn't resolved, can we put this on the agenda @rjackson?

@stefanpenner
Copy link
Member Author

resolution @rjackson and @stefanpenner will propose an upgrade path.

@rwjblue
Copy link
Member

rwjblue commented May 5, 2014

My suggested path is:

  • Detect usage of / and print deprecation warning. This would end up in 1.6.0.
  • Add feature flag (ember-routing-use-deprecated-slash-namespacing) that does the following (to be enabled by default in 1.7):
    • If 'true' (aka enabled), detect usage of '/', print deprecation warning and use old (current) behavior
    • If 'false' (aka disabled), do not replace / with ., no warnings/errors.
  • Change default value of feature flag. Making the old behavior opt-in. For 1.8.
  • Remove flag completed? 1.9?

@stefanpenner - +1/-1?

@stefanpenner
Copy link
Member Author

@rjackson I think @wycats would prefer no flag, just a hard deprecation.

@rwjblue
Copy link
Member

rwjblue commented May 5, 2014

OK, assuming no flagging, then the plan would be:

  • Detect usage of / and print deprecation warning, still do old behavior. This would end up in 1.6.0.
  • Detect usage of / and print deprecation warning, do not do old behavior. This would end up in 1.7.0.
  • Remove deprecation. 1.8.0.

@stefanpenner - Sound right?

@stefanpenner
Copy link
Member Author

@rjackson yes.
We should do another quick pass of other places where we enforce such hacks, but should be the responsibility of the resolver.

@stefanpenner
Copy link
Member Author

this also basically deprecates an early attempt at namespaces

@wagenet
Copy link
Member

wagenet commented Jul 28, 2014

See #5260

@stefanpenner
Copy link
Member Author

i'll start the deprecation process tonight

@stefanpenner
Copy link
Member Author

we are going to deprecate, this PR wil not be needed.

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.

5 participants