Conversation
…in the Check classes. Started with DiskFree and DiskUsage checks. - Changed the `data` value/structure so it's visible what kind of data is returned as well If acceptable I will continue with the other checks. Signed-off-by: Bram Leeda <bram@123inkt.nl>
| * @psalm-type ResultData array{availability: array{value: float, valueType: string}} | ||
| */ | ||
| class DiskFree extends AbstractCheck implements CheckInterface |
There was a problem hiding this comment.
I think CheckInterface should specify the psalm template here: not each concrete implementation.
Declaring an @template TResult of Failure|Success|Warning on the interface should highlight the missing @template-implements in each implementation of CheckInterface, and then we can remove all manual @return declarations on each of the check() methods.
WDYT?
There was a problem hiding this comment.
@Ocramius
That means that other code repositories with custom implementations of the interface, would also have failing psalm/phpstan checks after updating right?
It would be the better solution yes, and mean that all new checks would also set this up.
If the static-analysis issue is okay (don't know if it falls under a bc break, as the code api doesnt change) I will change this :)
There was a problem hiding this comment.
Yes, they would get a failure during analysis, but we don't really consider those BC breaks: the code works as it did before, and the types are just more precise.
There was a problem hiding this comment.
I would leave #62 alone for now :)
As for the patch: please rebase, instead of merging upstream.
|
I removed from milestone since PR is closed by author. |
4 participants
Add ResultInterface template for the
datafield, for strict typing in the Check classes.Started with DiskFree and DiskUsage checks.
datavalue/structure so it's visible what kind of data is returned as wellIf acceptable I will continue with the other checks.
Signed-off-by: Bram Leeda bram@123inkt.nl