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

Model cast count as integer #492

Merged

Conversation

daniel-aranda
Copy link

Not a big deal, but as I was writing some unit tests I figured this small glitch.

Test:

  $this->assertSame(3, App\Models\SomeModel::count());

Result:

Failed asserting that '3' is identical to 3.

@koenpunt
Copy link
Collaborator

Thanks, can you squash your commits?

@Rican7
Copy link
Collaborator

Rican7 commented Feb 16, 2015

Yea, a quick squash and this would be good to merge. Thanks for this!

@koenpunt
Copy link
Collaborator

And, is it necessary to assign it to the $count variable before returning?

Removing second return

unifying line
@daniel-aranda
Copy link
Author

Squash done at 8bfb68b.

I added the var because I like to separate tasks(thinking in readability), one task is to count a resultset and other to cast the result.

For readability is harder to understand a line with more than one task. For this particular case, this is not a biggie so I removed it.

Thanks for the review guys, please let me know any other issue.

Bests,

koenpunt added a commit that referenced this pull request Feb 16, 2015
@koenpunt koenpunt merged commit 53754c5 into jpfuentes2:1.1-dev Feb 16, 2015
@koenpunt
Copy link
Collaborator

Cool, thanks!

@Rican7
Copy link
Collaborator

Rican7 commented Feb 16, 2015

Thanks for the contribution and quick action! 👍

koenpunt added a commit that referenced this pull request Jun 3, 2017
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.

3 participants