-
Notifications
You must be signed in to change notification settings - Fork 84
Use doctrine/cs to ensure code standards of this project #155
Conversation
/** | ||
* @var string | ||
*/ | ||
/** @var string */ | ||
private $type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is safe to drop this write-only property in the class' constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it since the property is private and nothing else uses it.
} | ||
|
||
$success = $all ? $cacheProvider->deleteAll() : $cacheProvider->delete($cacheId); | ||
$color = $success ? 'info' : 'error'; | ||
$success = $success ? 'succeeded' : 'failed'; | ||
$message = null; | ||
|
||
if ( ! $all) { | ||
if (! $all) { | ||
$message = "Deletion of <$color>%s</$color> in <$color>%s</$color> has <$color>%s</$color>"; | ||
$message = sprintf($message, $cacheId, $cacheName, $success, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to remove these variables inside the string on line 55 without affect this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You would have to either move the variables down in to the sprintf
or use string concatenation.
@carusogabriel please don't be discouraged here, but with the potential deprecation of this bundle (see #156) I'd rather avoid doing too much cosmetic work just to throw it away. |
@alcaeus I have no problem in continue this one, but if this bundle is going to be deprecated, let's skip it 👍 |
Closing as per @alcaeus' comment. |
No description provided.