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

separate locals from config values #2648

Closed
wants to merge 1 commit into from

Conversation

aks-
Copy link

@aks- aks- commented May 14, 2015

Keeping a separate namespace so that what's sent per-request is separate from long-lived configuration values.

@aredridel
Copy link
Contributor

Oh, this is interesting, and super useful for the namespace collisions that have bothered users of adaro.

@dougwilson
Copy link
Contributor

As this is not backwards-compatible, it would have to wait for 5.0. I'm not against this at all, just highlighting why it won't get merged right away :) I'm a +1 for this, though.

@aredridel
Copy link
Contributor

That's awesome. The mashed-up namespaces there really do make backward compatibility a pain -- you can't safely add anything. Separating locals is the best start.

@aks-
Copy link
Author

aks- commented May 15, 2015

Aha, thank you folks!

@wesleytodd
Copy link
Member

I think the remaining things we need to do to land this are:

  1. A deprecation warning landed in 4.x for a key named locals for render
  2. Docs updates to address this change both in the upgrade instructions and in the docs for this method
  3. Tests

I know we are resurrecting a PR from 2015 here, so I dont expect folks here to volunteer, but if you want to work on this please volunteer soon. I will circle back this weekend and if no one has stepped up I will try and knock out these items.

@wesleytodd
Copy link
Member

Hey, I am guessing the folks in this PR are not interested in helping get this over the finish line at this point. Totally understandable, but also I think it means we are going to kick the can to v6 on this. We have limited contributor time and want to ship v5 asap so if we cannot get assistance landing these we are unable to get to them all. I hope we can change that in the future and start landing things more quickly for v6 and beyond.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants