Replace PrestaTrust property with setter #9451
Conversation
| * @param bool $enable | ||
| * @return $this | ||
| */ | ||
| public function toggleService($enable) |
There was a problem hiding this comment.
Not sure toggle is the right name when you are not toggling but setting a value.
There was a problem hiding this comment.
I have been looking for a name used for enabling OR disabling in the same method. Having two different methods (enable() / disable()) wasn't convenient here.
Suggestions are appreciated. :)
There was a problem hiding this comment.
setEnabled & isEnabled for accessor?
There was a problem hiding this comment.
+1 for setEnabled / isEnabled
There was a problem hiding this comment.
or something like configureActivation, setActivationStatus
There was a problem hiding this comment.
well rather setActivated in that case since it's a boolean
| public function toggleService($enable) | ||
| { | ||
| $this->enabled = (bool) $enable; | ||
| return $this; |
There was a problem hiding this comment.
Symfony convention recommends an empty line before any return statement
PierreRambaud
added
the
Waiting for QA
ntiepresta
added
QA ✔️
mickaelandrieu
force-pushed
the
replace-property-with-method
branch
from
fe5c214
to
a979fd2
Compare
|
Thanks @Quetzacoalt91 and everyone for review! |
This change is