-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Omnia/feature/#77 tax interface #62
Conversation
@pdbreen Can you take a look at this? |
|
||
public function hasBindings(array $bindings): self | ||
{ | ||
$this->bindings = array_merge($this->bindings, $bindings); |
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.
Like other types, bindings
should be declared as a class property, initialized to empty array.
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.
Since there is existing test coverage for TipoffPackage/TipoffServiceProvider, its best practice to extend the tests for the new functionality.
Also, one specific change needed.
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.
looks ok to me
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.
Thank you! 🌜
closes #61
Support package
-added TaxRequest interface
-added TaxRequestItem interface
-extended TipoffPackage with hasBindings helper
-added bindings to TipoffServiceProvider