Review: Needs Fixing

1. OMG code duplication: the entire contents of http-client.xq.src is copied 
(and modified?) from the original http-client module. That's nearly 3000 lines 
of some of the ugliest and most error-filled code we've got. No way do we want 
to maintain two copies of it. Also, they both call curl_global_init() and 
curl_global_cleanup(), which means that they will stomp on each other if anyone 
attempts to use both. This code must be consolidated somehow, perhaps into a 
common library that both modules use. Or, we need to simply eliminate the 
original http-client library. Perhaps we could replace the old http-client 
module with a pure XQuery wrapper module that replicates the old API, much like 
V2 of the EXPath http-client module does?

2. Need to consolidate the CMake code which searches for curl, etc. It was 
copied from modules/com/zorba-xquery/www/modules/CMakeLists.txt, and it still 
exists in that location. That means the search for CURL is done twice, and the 
cached variable ZORBA_HAVE_CURL is defined twice, and the Windows .pem files 
are copied twice... I would suggest that it should remain where it is in 
modules/CMakeList.txt, and be deleted from the other location, and testing done 
to ensure it still works right. (This issue would also be resolved by 
eliminating the original http-client module or replacing it with a wrapper 
module.)

3. The module error codes need to be changed to match the coding guidelines: 
http://my.zorba.io/dokuwiki/doku.php?id=coding-guidelines#error_codes


-- 
https://code.launchpad.net/~zorba-coders/zorba/json-http-module/+merge/169579
Your team Zorba Coders is subscribed to branch lp:zorba.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to     : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp

Reply via email to