Skip to content

Don't use php global#84

Open
imel96 wants to merge 1 commit intolcache:mainfrom
imel96:master
Open

Don't use php global#84
imel96 wants to merge 1 commit intolcache:mainfrom
imel96:master

Conversation

@imel96
Copy link
Copy Markdown

@imel96 imel96 commented Aug 10, 2017 •
edited
Loading

Replace $_SERVER with created time argument
Add phpcbf changes

Overview

This pull request:

  • Fixes a bug
  • Adds a feature
  • Breaks backwards compatibility
  • Needs tests

@imel96
Copy link
Copy Markdown
Author

imel96 commented Aug 10, 2017

Hi guys,

This pr is still failing a few tests. Just want to know the status of this project and if I should continue works on it? Cheers

@davidstrauss
Copy link
Copy Markdown
Member

davidstrauss commented Aug 10, 2017

This pr is still failing a few tests. Just want to know the status of this project and if I should continue works on it?

LCache is just on the back burner for me pending other projects. I think these are good changes, though.

@imel96 imel96 force-pushed the master branch 11 times, most recently from a732cc9 to 7dc8f2d Compare August 28, 2017 03:08
@imel96
Copy link
Copy Markdown
Author

imel96 commented Aug 28, 2017

So, we're integrating lcache with our app and we just needed to remove the php globals and on top of that we're adding a redis implementation as L2. We split the namespaces to separate L1 and L2.
Our redis patch would be based on this patch.
I see there's another PR here that's waiting to be merged, please let me know if I should rebase our patch on top of that one instead. Cheers

@imel96 imel96 force-pushed the master branch 4 times, most recently from 91b4da4 to 316e4b9 Compare August 29, 2017 20:25
@imel96 imel96 force-pushed the master branch 2 times, most recently from 9623310 to a499b5d Compare October 15, 2017 13:37
@imel96 imel96 force-pushed the master branch 2 times, most recently from 6e5f210 to 441d5d7 Compare October 27, 2017 05:38
@imel96
Copy link
Copy Markdown
Author

imel96 commented Oct 27, 2017

@davidstrauss some updates, since the original PR, I've updated the patch to use time(). The reason for that is there's no other way to calculate TTL correctly without using current time. I think this it's a bug. It can be easily triggered by introducing sleep() in user code. It was discovered as part of https://github.com/imel96/simple-lcache which implements PSR SimpleCache using Lcache. I've also fixed a few other bugs and now I can confidently run SimpleLcache on production

Don't use php globals
43a9197 
Replace requested time from $_SERVER with real time
Split namespaces
Add l1, l2 and state namespaces
Minor fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@imel96 @davidstrauss