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

Modernize JSON::RPC #31

Merged
merged 10 commits into from
Apr 24, 2017
Merged

Modernize JSON::RPC #31

merged 10 commits into from
Apr 24, 2017

Conversation

azawawi
Copy link
Contributor

@azawawi azawawi commented Jan 18, 2017

  • Modernize META.info to META6.json
  • Add Test::META tests
  • CI: Test on macOS and windows on 2016.12 and latest. Please enable AppVeyor support.
  • Add missing LICENSE file.

@bbkr
Copy link
Owner

bbkr commented Jan 18, 2017

Thanks, I'll review your changes during weekend (not much free time after $dayjob).

If we speak about modernization - this module was written in early Parrot VM era. It is full of antipatterns (exception based flow control, procedural design without proper requests isolation). And it is pretty much dead protocol - https://www.google.com/trends/explore?q=json-rpc,http%20rest .

I have to estimate time required for proper refactoring (with support of parallel batch processing of course and switch to more polished modules like JSON::Fast) and decide what will be the future of this module.

@azawawi
Copy link
Contributor Author

azawawi commented Jan 18, 2017

@bbkr Thanks for your time. I am using it for Odoo::Client so it is not dead as a protocol considering a big enterprise Odoo ERP is using it for integration work :)

@bbkr bbkr mentioned this pull request Jan 22, 2017
@bbkr
Copy link
Owner

bbkr commented Jan 22, 2017

Please take a look at refactoring roadmap #32 .
Comments are welcome.

I think this should take about 2-3 weeks, including applying changes from your MR and enabling AppVeyor.

@azawawi
Copy link
Contributor Author

azawawi commented Jan 22, 2017

Updated the roadmap. Thanks for your work 👍

@bbkr bbkr merged commit c30d542 into bbkr:master Apr 24, 2017
@azawawi
Copy link
Contributor Author

azawawi commented Apr 26, 2017

Thanks 👍

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.

2 participants