[Notifier] Bring consistency to bridges #50308
Conversation
| Q | A |
|---|---|
| Branch? | 6.3 |
| Bug fix? | no |
| New feature? | no |
| Deprecations? | no |
| Tickets | - |
| License | MIT |
| Doc PR | - |
| /** | ||
| * @return $this | ||
| */ | ||
| public function diffusionName(int $diffusionName): static |
There was a problem hiding this comment.
it's a bit weird to me that an option named "diffusionName" only accepts integers
can you please confirm this is correct @gnito-org?
|
The TL;DR of this PR is that options shouldn't have getters when they're not required, and their setters should not start with The rest is more CS-related. /cc @OskarStark @alamirault since you contribute to this component. 🙏 |
| throw new InvalidArgumentException(sprintf('The "From" number "%s" is not a valid phone number. The number must be in E.164 format.', $this->from)); | ||
| $options['from'] = $message->getFrom() ?: $this->from; | ||
| $options['to'] = [$message->getPhone()]; | ||
| $options['accountId'] ??= $this->accountId; |
There was a problem hiding this comment.
Before we used account_id kn the request, now accountId, is it correct or did I overlooked sth?
There was a problem hiding this comment.
The option array should be aligned with the API. The API uses accountId, that's why.
|
I'm merging to fix the CI and because this is ready to me, but feel free to comment after the merge if needed! |