-
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
big update to modern package versions, many changes to tests + more #1
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.
im not sure what this repo is meant to do, it feels a bit like overengineering. i like the sound of merging it into a main package though. could we potentially delete it all together if there's another better supported package we could drop in?
* @return $this | ||
*/ | ||
public function push($criteria) | ||
public function push(...$values) |
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.
ive never seen ... syntax in php, does it bundle all the args into a single value?
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.
it's the "splat" operator used for argument unpacking https://wiki.php.net/rfc/argument_unpacking
strategy: | ||
fail-fast: false | ||
matrix: | ||
php: [ 7.3 ] | ||
|
||
name: PHPUnit (PHP ${{ matrix.php }}) | ||
name: PHPUnit (PHP 7.3) | ||
|
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.
FYI this could have been used to run the tests on various PHP versions (eg. 7.4 & 8.0)
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.
oh hmmm... yeah forward-support's a nice idea. I'll add it, see if it passes out of the box, if so I'll keep otherwise I'll leave as-is
This should be tagged as a new major version.