-
Notifications
You must be signed in to change notification settings - Fork 9
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
Cleanup #6
Cleanup #6
Conversation
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.
Thanks for this PR! Looks pretty good so far 👍
src/Collections/Definitions.php
Outdated
* | ||
* @param string $name | ||
* | ||
* @return Schema |
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 would not add php docs not bringing new information, it just makes the code harder to read imo.
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.
True, I will remove it
src/Parameter.php
Outdated
@@ -114,6 +114,8 @@ public function getAllowEmptyValue() | |||
* Default value is `false`. | |||
* | |||
* @param bool $allowEmptyValue | |||
* | |||
* @return Parameter |
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.
What about adding php type hints instead?
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 thought about doing it. There are already places where they are used. But of course this will be without BC. So if it's OK with you I can add to all methods arguments and returns.
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 long as classes are final this is fine :)
src/Parameter.php
Outdated
@@ -39,7 +39,7 @@ | |||
/** @var bool|null */ | |||
private $allowEmptyValue; | |||
|
|||
public function __construct($data = []) | |||
public function __construct(array $data = []) |
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.
Please remove these, they're here on purpose to allow ArrayObject and stdClass.
src/Parts/RefPart.php
Outdated
@@ -34,6 +34,8 @@ public function getRef() | |||
|
|||
/** | |||
* @param string|null $ref | |||
* | |||
* @return RefPart |
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
Should I add type hints for the traits as well ? They are marked as internal |
Sure :) |
Thank you @dragosprotung! :) |
No description provided.