Improve DriverInterface #847
Conversation
Codecov Report
@@ Coverage Diff @@
## master #847 +/- ##
=========================================
Coverage 98.50% 98.50%
Complexity 345 345
=========================================
Files 23 23
Lines 1002 1002
=========================================
Hits 987 987
Misses 15 15 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| * @noinspection PhpMissingParamTypeInspection | ||
| * @noinspection PhpMissingReturnTypeInspection | ||
| * @noinspection ReturnTypeCanBeDeclaredInspection | ||
| * @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection |
There was a problem hiding this comment.
I've mixed feeling if this should be here: Disabling these inspections makes it nicer for the developer to focus on the bigger problems, but it's also basically hiding some problems.
There was a problem hiding this comment.
I don't think those should be added. They are phpstorm-specific and those inspections are not suited for any code that has to respect semver anyway (outside of major version) so they should do a better job at deciding when they report things. If you care about getting a focused feedback from phpstorm, you will have to disable those inspections when working on open-source code for any code that is already released.
| @@ -159,8 +167,8 @@ public function back(); | |||
| /** | |||
| * Sets HTTP Basic authentication parameters. | |||
| * | |||
| * @param string|false $user user name or false to disable authentication | |||
| * @param string $password password | |||
There was a problem hiding this comment.
Unfortunately, the IDE has this style rule not enabled by default.
If it's preferred to have this sort of style, I can roll it back - I'll drop in an .editorconfig rule for the future though.
There was a problem hiding this comment.
for now, please keep the alignment there to minimize the diff.
| * @noinspection PhpMissingParamTypeInspection | ||
| * @noinspection PhpMissingReturnTypeInspection | ||
| * @noinspection ReturnTypeCanBeDeclaredInspection | ||
| * @noinspection PhpElementIsNotAvailableInCurrentPhpVersionInspection |
There was a problem hiding this comment.
I don't think those should be added. They are phpstorm-specific and those inspections are not suited for any code that has to respect semver anyway (outside of major version) so they should do a better job at deciding when they report things. If you care about getting a focused feedback from phpstorm, you will have to disable those inspections when working on open-source code for any code that is already released.
| #[Language('XPath')] | ||
| $xpath, | ||
| $char, | ||
| #[ExpectedValues([null, 'ctrl', 'alt', 'shift', 'meta'])] |
There was a problem hiding this comment.
We already indicate that in the phpdoc in an interoperable way. so -1 for adding a jetbrains-only info about it (phpstorm is able to read this info from the phpdoc already).
There was a problem hiding this comment.
Ah, sorry I missed that. I'll roll it back. 👍
| @@ -159,8 +167,8 @@ public function back(); | |||
| /** | |||
| * Sets HTTP Basic authentication parameters. | |||
| * | |||
| * @param string|false $user user name or false to disable authentication | |||
| * @param string $password password | |||
There was a problem hiding this comment.
for now, please keep the alignment there to minimize the diff.
The changes are fully backword compatible.