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

Adding SizeCalculator support to GridFS #603

Merged
merged 1 commit into from
Jun 4, 2019

Conversation

bsperduto
Copy link
Contributor

Adds support for the SizeCalculator interface to the GridFS adapter.

Copy link
Contributor

@nicolasmure nicolasmure left a comment

Choose a reason for hiding this comment

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

thank you for your contribution ;)


public function size($key)
{
if(!$this->exists($key)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you apply the PSR2 coding style please ?

if(!isset($size['length'])) {
return false;
}
return $size['length'];
Copy link
Contributor

Choose a reason for hiding this comment

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

same about return statement; PSR2 says it should have an empty line before it ;)

public function testSize()
{
$this->filesystem->write('sizetest.txt', 'data');
$this->assertGreaterThan(0, $this->filesystem->size('sizetest.txt'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the returned value be equal to 4 (length of data) ?
You can run this test by putting your gridFS credentials in the phpunit.xml file (copied from the phpunit.xml.dist file).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I overthought it, it's changed

@bsperduto bsperduto force-pushed the rel/gridfs-size-calculator branch from d57c8af to 17313c5 Compare January 31, 2019 13:22
@nicolasmure
Copy link
Contributor

Thank you @bsperduto !

@nicolasmure nicolasmure merged commit 8d8d40c into KnpLabs:master Jun 4, 2019
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