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

Make each nunjucks environment have its own globals property #574

Merged
merged 3 commits into from
Nov 6, 2015
Merged

Make each nunjucks environment have its own globals property #574

merged 3 commits into from
Nov 6, 2015

Conversation

thepechinator
Copy link
Contributor

This is for the shared globals between environments issue mentioned in #573. Let me know if you'd like me to revise anything.

Here's what's included:

  • Added a test case.
  • Updated test/util.js's equal method to take in an optional env parameter, since globals aren't shared between environments anymore.
  • Updated Context in src/environment.js to take in an optional env parameter, for same reason.

Thanks!

- Add test case to check for this.
- Update util.js's equal method to take in an optional env parameter, since globals aren't shared between environments anymore.
- Update Context in src/environment.js to take in optional env parameter, for same reason.
@carljm
Copy link
Contributor

carljm commented Oct 30, 2015

Looks great! Can you also add a note to the CHANGELOG file? Thanks!

@thepechinator
Copy link
Contributor Author

Done. Thanks for the quick review.

@thepechinator
Copy link
Contributor Author

Had to fix conflicts with PR #575, which was merged into master. carljm, is there anything else I need to do before this can be merged into master?

@carljm
Copy link
Contributor

carljm commented Nov 6, 2015

Nope, looks good, merging! Sorry, must have missed the email notification on your update.

carljm added a commit that referenced this pull request Nov 6, 2015
Make each nunjucks environment have its own globals property
@carljm carljm merged commit e0cab39 into mozilla:master Nov 6, 2015
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.

2 participants