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

[5.4] Cache fix 15578 #15831

Merged
merged 2 commits into from
Oct 12, 2016
Merged

[5.4] Cache fix 15578 #15831

merged 2 commits into from
Oct 12, 2016

Conversation

aedart
Copy link
Contributor

@aedart aedart commented Oct 9, 2016

The changes are made, however, I'm not too sure about all the store's underlying implementation. Therefore, I would really appreciate your eyes on those changes and the tests that I have changed or added.

Also, because this is a change in the interface, I presumed that these change will not make it into the 5.3 series.

@GrahamCampbell GrahamCampbell changed the title Cache fix 15578 [5.4] Cache fix 15578 Oct 9, 2016
@GrahamCampbell
Copy link
Member

Please revert the change to the gitignore and squash to one commit.

Change flush return type

If a flush operation fails, then you now have chance of knowing about it.

Change flush method, return false on failure

Change flush test

Ensuring that flush method does return false, should the delete
directories return false.

Change wrapper flush return type

Both apcu_clear_cache() and apc_clear_cache() return bool values, which
can indicate if flush operation fails.

Change flush method, return bool

Add Apc flush test

Change flush method return type

Change flush test, check return of flush method

Change flush method return type

The table delete() returns the number of affected rows. If the number is
higher that zero, then the operation has succeeded. If not, it means that
there was nothing to delete... Not too sure about this one, however, it
should be the most correct response, if you do expect something to be
deleted and zero is returned.

Change flush test, check flush return

Change flush method return type

Memcached's flush method also returns true.

Add flush test

Change flush method return type

Change flush method return type

Not sure about flushdb() return. However, from what I understand, it never
fails and thus should return something like "ok" back.

Add flush test

Add release note about cache flush change

Revert "Ignore PHPStorm IDE file"

This reverts commit 8176c32.
@aedart
Copy link
Contributor Author

aedart commented Oct 9, 2016

Done - sorry about the "stupid" commit message. Used an IDE and pressed a bit too quickly though the wizard.

@taylorotwell
Copy link
Member

You don't need to update the change log manually. Please remove that.

@taylorotwell taylorotwell merged commit ddc3714 into laravel:master Oct 12, 2016
@aedart aedart deleted the cache-fix-15578 branch October 12, 2016 20:16
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.

3 participants