Skip to content
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

Type Hint WhereNotBetween #731

Merged
merged 6 commits into from
Dec 10, 2020
Merged

Type Hint WhereNotBetween #731

merged 6 commits into from
Dec 10, 2020

Conversation

amberlampsio
Copy link

Resolves issue,

Parameter #2 $values of method Illuminate\Database\Eloquent\Builder<Illuminate\Database\Eloquent\Model>::whereNotBetween() expects array<string, mixed>, array<int, string> given.

Changes

Type hint "int" for whereNotBetween methods

Breaking changes

No Breaking changes

@amberlampsio amberlampsio changed the title type hint Type Hint WhereNotBetween Dec 9, 2020
@szepeviktor
Copy link
Collaborator

Thank you.
Could you take a look at the tests?

@amberlampsio
Copy link
Author

@szepeviktor I am unable to retrigger tests on this PR -- perhaps you can? I'm not sure why this is failing either

@szepeviktor
Copy link
Collaborator

Done ⚙️

@szepeviktor
Copy link
Collaborator

How does PHPUnit's parseTestMethodAnnotations comes here?

@amberlampsio
Copy link
Author

How does PHPUnit's parseTestMethodAnnotations comes here?

I have no idea -- I'm investigating now, perhaps its the github actions matrix

Locally it's 👌
Screen Shot 2020-12-09 at 5 16 14 PM

@amberlampsio
Copy link
Author

How does PHPUnit's parseTestMethodAnnotations comes here?

So its a conflict in Laravel 6.x, OrchestraBench 4.12.1

@amberlampsio
Copy link
Author

@szepeviktor so I added a placeholder method into ApplicationResolver -- seems to solve the issue or we can try to filter out this specific version of Testbench

@szepeviktor
Copy link
Collaborator

Thank you :)

@canvural
Copy link
Collaborator

Hi,

Thank you for the PR! Can you also add a test for whereNotBetween with integer keyed array?

@amberlampsio
Copy link
Author

@szepeviktor I removed the parseTestMethodAnnotations method post update from testbench
@canvural Tests added 👍

@szepeviktor
Copy link
Collaborator

  • Locking orchestra/testbench (v4.12.2)
  • Locking orchestra/testbench-core (v4.11.1)

🎉

@canvural canvural merged commit ca98347 into larastan:master Dec 10, 2020
@canvural
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants