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

Presenter: removed static from refUrl variable #80

Merged
merged 1 commit into from
Jul 28, 2016

Conversation

Yrwein
Copy link
Contributor

@Yrwein Yrwein commented Jun 12, 2015

  • bugfix
  • documentation - not needed
  • BC break - no

The pull request fixes issue when testing presenters without isolation: You make two presenter calls with Nette\Http\Request-s on different domains - the first domain is cached for whole class (and not only instance) and so the second instance of the same presenter will redirect you to the first domain.
The test is appended mainly as example (there's no problem to remove it if it's not needed).

@Yrwein Yrwein force-pushed the remove_static_from_refUrl branch from bb63d7c to fb53532 Compare June 12, 2015 11:45
@fprochazka
Copy link
Contributor

👍 I like it. But how does it affect performance in real-world apps?

@Yrwein
Copy link
Contributor Author

Yrwein commented Jun 15, 2015

Frankly, I don't think there will be measurable performance hit from changing function's static variable to property. Or is there something I'm missing? (Nevertheless we can wait few weeks until it's deployed.)

@dg dg force-pushed the master branch 9 times, most recently from e9c215d to 99249bf Compare May 6, 2016 16:29
@dg dg force-pushed the master branch 5 times, most recently from b7a311f to eba19f3 Compare May 13, 2016 17:10
@dg dg force-pushed the master branch 6 times, most recently from 7b1ec30 to 3adfa43 Compare May 23, 2016 01:09
@dg dg force-pushed the master branch 4 times, most recently from c73b255 to 9e7cd60 Compare June 7, 2016 10:30
@dg dg force-pushed the master branch 2 times, most recently from ea11e9d to 8ff194f Compare June 16, 2016 14:57
@rostenkowski
Copy link

Hi there:)

what's the current status of this pull request?

@dg dg merged commit 2c86258 into nette:master Jul 28, 2016
@rostenkowski
Copy link

Thank you
👍

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