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

Custom cache functions #216

Closed
wants to merge 21 commits into from
Closed

Custom cache functions #216

wants to merge 21 commits into from

Conversation

peter-mw
Copy link
Contributor

As of #212

@peter-mw
Copy link
Contributor Author

Cant understand why the build fails. Have no reason to do so, can you see?

@peter-mw
Copy link
Contributor Author

Ok will fix indent issues

@treffynnon
Copy link
Collaborator

Looks like the build is failing with this error: https://travis-ci.org/j4mie/idiorm/jobs/28039970#L54

PHP Fatal error: Using $this when not in object context in /home/travis/build/j4mie/idiorm/test/CacheTest.php on line 43

@peter-mw
Copy link
Contributor Author

Got it, thanks for your time helping out.

@peter-mw
Copy link
Contributor Author

Hi Simon, the cache is not automatically deleted on save and this test never runs

https://github.com/peter-mw/idiorm/blob/master/test/CacheTest.php#L60

I want to make the cache to be deleted automatically on save. What do you think?

@treffynnon
Copy link
Collaborator

Well, that would change existing behaviour so that has the potential to
break backwards compatibility unless it were configurable and disabled by
default.

On 20 June 2014 15:08, Peter Ivanov notifications@github.com wrote:

Hi Simon, the cache is not automatically deleted on save and this test
never runs

https://github.com/peter-mw/idiorm/blob/master/test/CacheTest.php#L60

I want to make the cache to be deleted automatically on save. What do you
think?


Reply to this email directly or view it on GitHub
#216 (comment).

Simon Holywell
simonholywell.com
http://www.simonholywell.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature
functionalphp.com
http://www.functionalphp.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature
twitter.com/treffynnon
github.com/treffynnon

I have also written a book: Functional Programming in PHP
http://www.functionalphp.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature

@peter-mw
Copy link
Contributor Author

Added the option and readme.

It says the build is still failing, but i don't know why?

What i am doing wrong on line 47? I don't see https://github.com/peter-mw/idiorm/blob/master/test/CacheTest.php#L47

@treffynnon
Copy link
Collaborator

What is the error message you're getting back?

On 20 June 2014 15:58, Peter Ivanov notifications@github.com wrote:

Added the option and readme.

It says the build is still failing, but i don't know why?

What i am doing wrong on line 47? I don't see
https://github.com/peter-mw/idiorm/blob/master/test/CacheTest.php#L47


Reply to this email directly or view it on GitHub
#216 (comment).

Simon Holywell
simonholywell.com
http://www.simonholywell.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature
functionalphp.com
http://www.functionalphp.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature
twitter.com/treffynnon
github.com/treffynnon

I have also written a book: Functional Programming in PHP
http://www.functionalphp.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature

@peter-mw
Copy link
Contributor Author

Strange one.

Parse error: syntax error, unexpected T_FUNCTION in /home/travis/build/j4mie/idiorm/test/CacheTest.php on line 47

It works on my end and tests are passing

@treffynnon
Copy link
Collaborator

That is from a PHP 5.2 build - anonymous functions weren't supported back
then so please make your tests only run in later PHP versions by changing
the test class file name from *Test.php to *Test53.php just the like Config
tests have.

On 20 June 2014 16:05, Peter Ivanov notifications@github.com wrote:

Strange one.

Parse error: syntax error, unexpected T_FUNCTION in
/home/travis/build/j4mie/idiorm/test/CacheTest.php on line 47


Reply to this email directly or view it on GitHub
#216 (comment).

Simon Holywell
simonholywell.com
http://www.simonholywell.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature
functionalphp.com
http://www.functionalphp.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature
twitter.com/treffynnon
github.com/treffynnon

I have also written a book: Functional Programming in PHP
http://www.functionalphp.com/?utm_source=Email%20Signature&utm_medium=Email&utm_campaign=Email%20Signature

@peter-mw
Copy link
Contributor Author

I think its ready. Can you take a look

@peter-mw
Copy link
Contributor Author

Yeah fixed the spaces :)

Thanks for the guidelines, the implementation was not hard at all.

Hope this is OK to get merged at some time, so i can base my other work on it.

@peter-mw
Copy link
Contributor Author

Done

@peter-mw
Copy link
Contributor Author

Wait i need to override the create_cache_key function too, will add it now

@peter-mw
Copy link
Contributor Author

Think its OK now.

I will try to implement the cache in the real app.

Will test properly and update this issue if i find any bugs

@ghost
Copy link

ghost commented Jun 21, 2014

In the config docs

Idiorm's cache in never cleared by default

should be

Idiorm's cache is never cleared by default

Also, should this be merged into develop first?

@treffynnon
Copy link
Collaborator

Yes, this will be merged into the develop branch first. I can do this even
if the pull request is against master - it's just more difficult.

@peter-mw
Copy link
Contributor Author

Hi, i can send PR in the develop branch, just the develop version was not working , that's why i pulled master

@treffynnon
Copy link
Collaborator

Merged in commit e7b77ad

@treffynnon treffynnon closed this Jun 22, 2014
@peter-mw
Copy link
Contributor Author

YoHoo! Thanks for the release :)

@peter-mw peter-mw mentioned this pull request Nov 18, 2014
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