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.

Sign up for GitHub

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

Jump to bottom

Update taggable cache and tests #4

Merged
merged 3 commits into from
Jan 25, 2016
Merged

Update taggable cache and tests #4

merged 3 commits into from
Jan 25, 2016

Conversation

prisis
Copy link
Contributor

@prisis prisis commented Jan 25, 2016

No description provided.

@Nyholm
Copy link
Member

Nyholm commented Jan 25, 2016

Thank you for this PR.
Have a look at the Redis adapter and its Readme. We would like all readmes to follow that template where it's possible.

@codecov-io
Copy link

Current coverage is 100.00%

Merging #4 into master will not affect coverage as of 03981c8

@@            master      #4   diff @@
======================================
  Files            1       1       
  Stmts            8       8       
  Branches         0       0       
  Methods          4       4       
======================================
  Hit              8       8       
  Partial          0       0       
  Missed           0       0

Review entire Coverage Diff as of 03981c8

Powered by Codecov. Updated on successful CI builds.

@prisis
Copy link
Contributor Author

prisis commented Jan 25, 2016

Hey @Nyholm so i should remove the Feature | Supported table?

@Nyholm
Copy link
Member

Nyholm commented Jan 25, 2016

Yeah, we have that table on www.php-cache.com instead.

@prisis
Copy link
Contributor Author

prisis commented Jan 25, 2016

Okay, so i can change it on all repositories?

the composer.json info need to be removed too?

And dont merge this, need to squash it

@Nyholm
Copy link
Member

Nyholm commented Jan 25, 2016

That would be awesome.
There is a ticket for that: php-cache/issues#10

Aslo, @aequasi is about to update the URL to the issues, see: php-cache/issues#24 I think that you can use the new URL if you are about to change all repos.

Nyholm
Nyholm reviewed Jan 25, 2016
View reviewed changes
README.md
@@ -4,8 +4,28 @@
This is a void implementation of PSR-6. Other names for this adapter could be Blackhole or Null. This
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to include the badges

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And

This is a PSR-6 cache implementation for Redis. It is a part of the PHP Cache organisation. To read about features like tagging and hierarchy support please read the shared documentation at www.php-cache.com.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nyholm is it okay it they all flat-square and come from shields.io?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested Shields.io just now. It is way slow and seams to fail.
screen shot 2016-01-25 at 16 53 14

I would suggest to copy from the redis adapter and make a search replace.

[![Latest Stable Version](https://poser.pugx.org/cache/redis-adapter/v/stable)](https://packagist.org/packages/cache/redis-adapter) [![codecov.io](https://codecov.io/github/php-cache/redis-adapter/coverage.svg?branch=master)](https://codecov.io/github/php-cache/redis-adapter?branch=master) [![Build Status](https://travis-ci.org/php-cache/redis-adapter.svg?branch=master)](https://travis-ci.org/php-cache/redis-adapter) [![Total Downloads](https://poser.pugx.org/cache/redis-adapter/downloads)](https://packagist.org/packages/cache/redis-adapter)  [![Monthly Downloads](https://poser.pugx.org/cache/redis-adapter/d/monthly.png)](https://packagist.org/packages/cache/redis-adapter) [![Software License](https://img.shields.io/badge/license-MIT-brightgreen.svg?style=flat-square)](LICENSE)

@prisis
Copy link
Contributor Author

prisis commented Jan 25, 2016

Okay, i will do it today :)

@Nyholm
Copy link
Member

Nyholm commented Jan 25, 2016

You are awesome. Thanks!

cryptiklemur
cryptiklemur reviewed Jan 25, 2016
View reviewed changes
tests/IntegrationPoolTest.php
@@ -23,6 +23,11 @@ class IntegrationPoolTest extends BaseTest
'testHasItem' => 'Void adapter does not save,',
'testDeleteItems' => 'Void adapter does not save,',
'testSave' => 'Void adapter does not save,',
'testSaveWithoutExpire' => 'Void adapter does not save,',
'testDataTypeFloat' => 'Void adapter does only output boolean,',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Void adapter only outputs boolean

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for the other 3

@prisis prisis force-pushed the update-taggable-cache-and-tests branch 2 times, most recently from fd7d9d7 to 2640e83 Compare January 25, 2016 18:56
@prisis
Copy link
Contributor Author

prisis commented Jan 25, 2016

@Nyholm and @aequasi now you can check it again :)

Should i update cache/adapter-common to support only v0.2 or both?

cryptiklemur
cryptiklemur reviewed Jan 25, 2016
View reviewed changes
tests/IntegrationPoolTest.php
@@ -23,6 +23,11 @@ class IntegrationPoolTest extends BaseTest
'testHasItem' => 'Void adapter does not save,',
'testDeleteItems' => 'Void adapter does not save,',
'testSave' => 'Void adapter does not save,',
'testSaveWithoutExpire' => 'Void adapter does not save,',
'testDataTypeFloat' => 'Void adapter only output boolean,',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Void adapter only outputs boolean

Missed the s in outputs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry :)

@Nyholm
Copy link
Member

Nyholm commented Jan 25, 2016

Should i update cache/adapter-common to support only v0.2 or both?

If you like to. But there is some work to that. Examples: php-cache/apc-adapter@9200e21 and php-cache/filesystem-adapter@b3444f9

Nyholm
Nyholm reviewed Jan 25, 2016
View reviewed changes
composer.json
@@ -30,12 +30,12 @@
"php": "^5.5|^7.0",
"psr/cache": "1.0.0",
"cache/adapter-common": "^0.1",
"cache/taggable-cache": "^0.2"
"cache/taggable-cache": "^0.2|^0.3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use tagging ^0.3 and drop 0.2 completely.

@prisis
Copy link
Contributor Author

prisis commented Jan 25, 2016

@Nyholm done.

@prisis prisis force-pushed the update-taggable-cache-and-tests branch from c7ee3bb to fe70b3b Compare January 25, 2016 19:21
@prisis
update composer
c37d758 
Update readme doc

Update composer

to use cache/integration-tests v0.7 not dev-master

Skip more tests

remove table

fix spelling mistakes | update doc

	modified:   README.md

remove line

	modified:   README.md

Update readme doc

Update composer

to use cache/integration-tests v0.7 not dev-master

Skip more tests

remove table

fix spelling mistakes | update doc

	modified:   README.md

remove line

	modified:   README.md

adding missing s

	modified:   README.md

update composer php-cache/issues/issues/23

next small upadtes

	modified:   README.md
	modified:   composer.json
	modified:   tests/IntegrationPoolTest.php

	modified:   README.md

	modified:   composer.json
@prisis prisis force-pushed the update-taggable-cache-and-tests branch from fe70b3b to c37d758 Compare January 25, 2016 19:23
Nyholm
Nyholm reviewed Jan 25, 2016
View reviewed changes
composer.json
"cache/adapter-common": "^0.1",
"cache/taggable-cache": "^0.2"
"psr/cache": "~1.0",
"cache/adapter-common": "^0.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BC braking feature in 0.2 is that fetchObjectFormCache must return an array. You should do like this:

protected function fetchObjectFromCache($key)
{
    return [false, null];
}

cryptiklemur added a commit that referenced this pull request Jan 25, 2016
@cryptiklemur
Merge pull request #4 from PrisisForks/update-taggable-cache-and-tests
45cabcd 
Update taggable cache and tests
@cryptiklemur cryptiklemur merged commit 45cabcd into php-cache:master Jan 25, 2016
@prisis prisis deleted the update-taggable-cache-and-tests branch January 25, 2016 21:11
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.
4 participants
@prisis @Nyholm @codecov-io @cryptiklemur