-
-
Notifications
You must be signed in to change notification settings - Fork 478
phpstan-dba: improve coverage #1232
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
Conversation
composer.lockPackage changes
Dev Package changes
Settings · Docs · Powered by Private Packagist |
/** | ||
* @param string $name | ||
* @param int $offset | ||
* @param int $limit |
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.
you could use actual parameter types rather than phpdoc
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.
Yes please go wild with PHP 8 syntax in this repo 😄
The composer.lock diff comment has been updated to reflect new changes in this PR. |
The composer.lock diff comment has been updated to reflect new changes in this PR. |
@@ -469,6 +469,9 @@ public function getDependents(string $name, int $offset = 0, int $limit = 15, st | |||
return $this->getEntityManager()->getConnection()->fetchAllAssociative($sql, $args); |
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.
Interesting failure here, it seems to pick up the conditional d.total usage but not the join declaring it? TBH this is quite messy code and I don't expect perfect handling here, but it should at least ignore it rather than error I guess.
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.
will investigate in staabm/phpstan-dba#188
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.
Cool, no rush, glad to provide this repo as playground :) I have another project with hundreds of pure SQL queries I could run the extension on once it matured a bit, but sadly it's private.
src/Package/Updater.php
Outdated
* - id (version id, can be null for newly created versions) | ||
* - version (normalized version from the composer package) | ||
* - object (Version instance if it was updated) | ||
* @return array{updated: bool, id: int|null, version: string, object: Version|null} |
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.
Maybe better would be array{updated: true, id: int|null, version: string, object: Version}|array{updated: false, id: int|null, version: string, object: null}
. It's a bit messy with duplication but it would fix the error above I hope.
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.
ok, lets see whether this works
The composer.lock diff comment has been updated to reflect new changes in this PR. |
The composer.lock diff comment has been updated to reflect new changes in this PR. |
The composer.lock diff comment has been updated to reflect new changes in this PR. |
The composer.lock diff comment has been updated to reflect new changes in this PR. |
The composer.lock diff comment has been updated to reflect new changes in this PR. |
The composer.lock diff comment has been updated to reflect new changes in this PR. |
No description provided.