Skip to content

Remove useless try/catch in the CachedTrait #1931

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

Merged
merged 1 commit into from
May 3, 2018

Conversation

meyerbaptiste
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

Sorry, I missed the #1908 review.

@soyuka soyuka merged commit fb5adf1 into api-platform:2.2 May 3, 2018
@soyuka
Copy link
Member

soyuka commented May 3, 2018

thanks @meyerbaptiste

try {
$cacheItem->set($value);
$this->cacheItemPool->save($cacheItem);
} catch (CacheException $e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I was only copying exciting code in the previous pr so I am not sure of the original author's thoughts, but you are now keeping the exception handling above but removing this one. Should we be consistent?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no chance of CacheException being thrown here. The original author (i.e. myself) was mistaken. 😆

Copy link
Member

Choose a reason for hiding this comment

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

And we all copied without thinking twice 😆

@meyerbaptiste meyerbaptiste deleted the refactor_cached_trait branch May 14, 2018 09:00
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.

5 participants