- The indentation has several problems. Could you please make it consistent.
Especially in the C++ code.
sure.
- Why don't you use the same approach as we did for xbrl-cloud and keep the
connection map in JSONiq (using the map module)? I think that would be more
appropriate for this module
Review: Resubmit
External module merge proposal in
lp:~zorba-coders/zorba/feature-external-cloudant-module
and
lp:~zorba-coders/zorba/cloudant-module
--
https://code.launchpad.net/~zorba-coders/zorba/feature-cloudant/+merge/177111
Your team Zorba Coders is subscribed to branch lp:zorba.
--
- The indentation has several problems. Could you please make it consistent.
Especially in the C++ code.
- Why don't you use the same approach as we did for xbrl-cloud and keep the
connection map in JSONiq (using the map module)? I think that would be more
appropriate for this module than
I did not find a description of the process necessary to propose a new module
in the wiki.
What should I do?
Thanks
--
https://code.launchpad.net/~zorba-coders/zorba/feature-cloudant/+merge/177111
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
There isn't really a fixed process for that since it happens so rarely. Here's
what I'd suggest:
1. Create the module as a stand-alone directory, so that it works when located
in the zorba_modules directory as part of a Zorba build.
2. Push that module to Launchpad as
Review: Needs Information
The module itself looks OK at a quick glance. However, unless I'm missing
something, I don't think this needs to be a core module. It should be a
non-core module in a separate branch.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-cloudant/+merge/177111
Given the comments I received for the http-module I fear that the way I am
disconnecting from cloudant is not correct, because curl_global_init/cleanup
must not be called multiple times from different modules.
I tried disconnecting invoking an XQuery function but this does not work
because the
Review: Needs Fixing
Actually, a few code problems:
1. ConnectionMap::destroy() invokes a function in the
http://28msec.io/modules/cloudant namespace. I think that should be in
http://zorba.io/modules/cloudant, right?
2. The files in cloudant.xq.src have 28msec copyright notices.
3. I think
Ahh, I see, that call in ConnectionMap::destroy() is commented out. Didn't
notice that the first time.
Regarding the curl disconnect problems: That would all go away if this module
used the http-client module, rather than using C++ to invoke CURL directly. Is
there a reason this module
Hi Chris,
thanks for the quick review.
I think it is currently a core module due to the auto-disconnect functionality.
External module can have cpp external functions?
However, as I wrote in my previous comment, this feature is almost certainly
wrong at the moment.
If I recall correctly,
Actually, a few code problems:
1. ConnectionMap::destroy() invokes a function in the
http://28msec.io/modules/cloudant namespace. I think that should be in
http://zorba.io/modules/cloudant, right?
This is commented out :) I tried doing it in XQuery but I didn't manage so I
left the code
Ahh, I see, that call in ConnectionMap::destroy() is commented out. Didn't
notice that the first time.
Regarding the curl disconnect problems: That would all go away if this module
used the http-client module, rather than using C++ to invoke CURL directly. Is
there a reason this module
Yes, non-core modules can have C++ external functions; take a look at the
process module, for instance.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-cloudant/+merge/177111
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Review: Needs Information
Since in its current state this module depends on CURL i made a single search
for the
CURL library and moved both the ADD_DIRECTORY(http-client) and
ADD..(cloudant) inside
the same check. The http client cmakelist doesn't do the search again.
Aha! Now I see what
I agree that an external module is best suited for this module.
I will look in the wiki to see the submit procedure.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-cloudant/+merge/177111
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
15 matches
Mail list logo
