search

georgique / yii2-jsonrpc

Yii2 JSON-RPC server implementation based on method-to-action translation.
BSD 2-Clause "Simplified" License
9 stars 8 forks source link

Feature refactoring #4

Closed xzag closed 6 years ago

xzag commented 6 years ago

Hi I've made a few updates to the code. Mostly adapting php 7 approach to handling exceptions via Throwable interface. Also made some fixes to improve JSON-RPC specs compatibility regarding notifications and batch requests

Next thing is demo yii2 app with codeception tests. Postman requests collection is also available

Just for fun included travis config for this branch

Build Status

On the downside though, my updates are setting PHP version constaint >= 7.0. Which might be inapropriate for someone

Let me know what you think

georgique commented 6 years ago

Hey @xzag! Thanks for such a great contribution! However, I haven't reviewed the code changes yet, so the quick question is - how hard would it be to make it compatible with PHP 5.6 at least? It's still widely used, and I'm not sure I'm ready to give it up...

xzag commented 6 years ago

For PHP 5.6 we have to omit \Throwable and return ErrorHandler which you had earlier for catching Errors.

georgique commented 6 years ago

Can you please do that?

xzag commented 6 years ago

Yes, I'll update the code. Will update this PR when done

georgique commented 6 years ago

I've changed base for this PR, so it goes to develop branch instead of master, and now we have a bit of a conflict, becase I've done some further development too. I've resolved obvious conflicts, but some code changes are still needed. Going to work on that over the weekend, then will complete the merge. Then it will be in develop branch for some time, until tested, then we can issue a new version. Thanks again @xzag for your contribution.

xzag commented 6 years ago

sounds good. thanks

xzag commented 6 years ago

hi. any update?

georgique commented 6 years ago

Hi, yeah, sorry - I'm on a vacation now, going to work on this next week. Sorry for the delay.

georgique commented 6 years ago

@xzag I've merged it into a feature branch, and also added latest feature from develop branch there. Can you please review it and test? I might have time to test too later this week, so we can proceed with the new version. And yeah - thanks again for the improvement. I like the "model" approach for handling request.