-
Notifications
You must be signed in to change notification settings - Fork 10
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
feat: setup phpstan & rector #89
Conversation
498b5be
to
53c908c
Compare
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.
🤞
$this->sendErrorEvent($request, $reason); | ||
return $response; | ||
}, | ||
function (\Exception $reason) use ($request): never { |
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.
I didn't know that we could define a return type on a closure... and I'm not sure to understand how it could be a good thing to do so... but I guess we can let rector do whatever it wants 🤷
|
||
/** @var string */ | ||
protected $clientId; | ||
protected $events = []; |
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.
suggestion:
protected $events = []; | |
protected array $events = []; |
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.
As it's a rector change, I won't touch this (might be a BC break if this property is badly used -- maybe this should be in a new PR)
|
||
/** | ||
* Constructor | ||
* | ||
* @param string $clientId | ||
*/ | ||
public function __construct(EventDispatcherInterface $eventDispatcher, $clientId) | ||
public function __construct(EventDispatcherInterface $eventDispatcher, protected $clientId) |
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.
suggestion:
public function __construct(EventDispatcherInterface $eventDispatcher, protected $clientId) | |
public function __construct(protected EventDispatcherInterface $eventDispatcher, protected string $clientId) |
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.
I've applied the 1st change (a524310) but not the 2nd as it might be a BC break (could be done in a new PR I suppose, but I don't want to risk it now)
.github/workflows/ci.yml
Outdated
# TODO add --prefer-lowest once we're ready to handle it | ||
fail-fast: false | ||
steps: | ||
- uses: actions/checkout@master |
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.
- uses: actions/checkout@master | |
- uses: actions/checkout@v4 |
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.
Done 64753a1
0126cff
to
0c247de
Compare
53c908c
to
8bef10f
Compare
Why?
$quality += 2
How?
Setup PHPStan & Rector without caring about fixing everything : that will come later