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

fix Str::startsWith need string error #14

Merged
merged 3 commits into from Dec 6, 2019
Merged

Conversation

weretyczx
Copy link

@weretyczx weretyczx commented Dec 4, 2019

Hi there,
First of all, thanks u to spend a lot of time building this awesome package, it really allay the pain to generate api document.
But I found some problem here at version "mtrajano/laravel-swagger": "^0.6.0", it cause by the commend php artisan laravel-swagger:generate > swagger.json
螢幕快照 2019-12-04 下午2 32 26

GeneratesFromRules.php
螢幕快照 2019-12-04 下午2 47 59
The reason is the $rule variable have pass the object (Laravel Rule Object) getting exception, and I already fix this issue, please check up the code below before merge, thanks.

p.s. My editer auto format the elseif to else if and I forgot to ignore the change, i haven't change at that line

@mtrajano
Copy link
Owner

mtrajano commented Dec 5, 2019

Hello thank you for the for the pr, I will take a look at it tonight. In the meantime would you mind fixing the elseif (I need to add a linter to the repo) and adding a test to validate the fix? Thanks once again!

@weretyczx
Copy link
Author

Hello thank you for the for the pr, I will take a look at it tonight. In the meantime would you mind fixing the elseif (I need to add a linter to the repo) and adding a test to validate the fix? Thanks once again!

Sure, I already recovered else if style and send it to pr.

@mtrajano
Copy link
Owner

mtrajano commented Dec 6, 2019

Made a small change to not break Rules that implement __toString, since they are objects but you may still use string functions on them ex: https://laravel.com/docs/5.7/validation#rule-in and added tests as well. Great job, thanks for the pr!

@mtrajano mtrajano merged commit 4b07bd8 into mtrajano:master Dec 6, 2019
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.

None yet

3 participants