Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Cezar Andrei
It's strange that a few tests where failing because pdfbox was trying to use 
lob4j. The website clearly says it depends on commons-logging which can use 
log4j if available but fall down to java's default logging api.

I added explicit checks for jempbox and commons-lib to read-pdf module. And 
also added them to classpath.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126568
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Zorba Build Bot
Attempt to merge into lp:zorba failed due to conflicts: 

text conflict in ChangeLog
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126568
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Cezar Andrei
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126743
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Cezar Andrei
Review: Approve

Fixed fonts on build box and changed tests to be machine independent.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126743
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Matthias Brantner
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126743
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Zorba Build Bot
Attempt to merge into lp:zorba failed due to conflicts: 

text conflict in modules/ExternalModules.conf
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126743
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Cezar Andrei
Review: Approve

Merged and solved conflict.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126822
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Zorba Build Bot
Attempt to merge into lp:zorba failed due to conflicts: 

text conflict in modules/ExternalModules.conf
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126822
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Cezar Andrei
Review: Approve

Merged and solved conflict.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126823
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-27 Thread Cezar Andrei
Review: Approve

Now with tag for read-pdf.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126824
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-26 Thread Cezar Andrei
Review: Approve

I added texts for all functions.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-26 Thread Zorba Build Bot
Attempt to merge into lp:zorba failed due to conflicts: 

text conflict in ChangeLog
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-26 Thread Cezar Andrei
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126568
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-26 Thread Zorba Build Bot
The attempt to merge lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba 
failed. Below is the output from the failed tests.


CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:274 
(message):
  Validation queue job fread-pdf-trunk-2012-09-26T23-05-52.544Z is finished.
  The final status was:

  

  13 tests did not succeed - changes not commited.


Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126568
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-26 Thread Zorba Build Bot
The attempt to merge lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba 
failed. Below is the output from the failed tests.


CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:274 
(message):
  Validation queue job fread-pdf-trunk-2012-09-27T01-51-44.419Z is finished.
  The final status was:

  

  7 tests did not succeed - changes not commited.


Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/126568
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Matthias Brantner
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve  1, Disapprove  1, 
Needs Fixing  1, Pending  1. Got: 2 Approve, 1 Pending.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Chris Hillery
Cezar - The remote queue is still not finding the jars, although they are 
installed correctly in /usr/share/java as requested. (This is also probably why 
the experimental build isn't including the module.) The problem, I believe, is 
that you are using FIND_FILE() to find them, and you only list a few specific 
version numbers to find - I installed 1.7.1 from the URL you sent, but that 
isn't one of the versions listed in CMakeLists.txt in the read-pdf module.

The correct solution is to use ZORBA_FIND_JAR(), which will look for .jar files 
with any embedded version number. See 
src/com/zorba-xquery/www/modules/CMakeLists.txt in the data-formatting module 
for an example. If you make that change in the module, this merge proposal 
should succeed.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Chris Hillery
I have re-assigned my review request to Juan, since I will not be around much 
longer to vote on this proposal.

Juan, all you need to review is the CMake stuff in the lp:zorba/read-pdf-module 
branch, specifically to ensure that it uses FIND_ZORBA_JAR() appropriately. 
Once Cezar makes that change, you can vote Approve on this proposal.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Zorba Build Bot
The attempt to merge lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba 
failed. Below is the output from the failed tests.


CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:274 
(message):
  Validation queue job fread-pdf-trunk-2012-09-25T23-51-47.915Z is finished.
  The final status was:

  

  No tests were run - build or configure step must have failed.

  Not commiting changes.


Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Chris Hillery
The read-pdf module was found and built with this latest RQ run.

However, it has no tests. I personally don't think that's acceptable, even for 
a beta release.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-25 Thread Zorba Build Bot
The attempt to merge lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba 
failed. Below is the output from the failed tests.


CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:274 
(message):
  Validation queue job fread-pdf-trunk-2012-09-26T04-11-47.786Z is finished.
  The final status was:

  

  1 tests did not succeed - changes not commited.


Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake

-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-22 Thread Cezar Andrei
See answers inline:
- the error seems to be too general, essentially it always raises 
JAVA-EXCEPTION no matter what goes wrong (e.g. it the given input is not a 
valid pdf)
I adapted the error msg to be more clear/specific.

- the java stack trace seems to be sent to standard error
Goes to std err.

- Renders the each page of the PDF document as an image. = Renders each page 
of the PDF document as an image.
Done.

- the names of the private functions should also adhere to the code conventions 
renderToImages = render-to-images
Done.

- make xqdoc failes because the comments seem to contain invalid xml
/home/mbrantner/zorba/build/URI_PATH/com/zorba-xquery/www/modules/project_xqdoc.xq:142,9:
 user-defined error [err:UE004]: Error processing module zerr:ZXQD0002 -  This 
module provides funtionality to read the text from PDF documents and
 to render PDF documents to images.
 a href=http://pdfbox.apache.org;Apache PDFBox/a library is used to
 implement these functions.
 br /
 br /
 bNote:/b Since this module has a Java library dependency a JVM required
 to be installed on the system. For Windows: jvm.dll is required on the system
 path ( usually located in C:\Program Files\Java\jre6\bin\client.
 bNote:b For Debian based Linux distributions install PdfBox and FontBox
 packages: sudo apt-get install libpdfbox-java libfontbox-java
: can not parse as XML for xqdoc: loader parsing error: Opening and ending tag 
mismatch: b line 0 and root
; raised at 
/home/mbrantner/zorba/sandbox/src/runtime/errors_and_diagnostics/errors_and_diagnostics_impl.cpp:81
Done.

- adapt the year in Copyright 2006-2009 The FLWOR Foundation. in the .xq file 
(and some other files also)
Done.

- would it make sense to return one string per page in the pdf instead of one 
big string?
The API doesn't alow it, but I added two more optional options, to insert a 
user defined string at the start and end of each page.

- remove commented out code in read-pdf.cpp
Done.

- valgrind shows tons of invalid writes. Why? Are they critical? Is there 
anything we can do?
Jvm always shows in valgrind, even if nothing is done with it. I was careful to 
remove any allocated memory.

- would it make sense to return the images in a streaming fashion (i.e. don't 
create all base64's in a vector)?
No, because it's a push write of all images. And as discussed, optimize only a 
copy in some cases isn't worth the effort.

- encoding each image shouldn't be necessary and will probably we wasted effort 
because the images might be written to a file in their binary form
Done.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-22 Thread Cezar Andrei
Review: Approve


-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-22 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve  1, Disapprove  1, 
Needs Fixing  1, Pending  1. Got: 1 Approve, 2 Pending.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125858
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-21 Thread Paul J. Lucas
It doesn't look like the fpdf.* files were added via bzr add -- were they 
supposed to be?
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-21 Thread Cezar Andrei
Paul,

If you're asking about my last commit msg, it is as it should be just one line 
change, by deleting the tag name, it makes trunk use the latest HEAD version of 
the module branch. The files in util-jvm were already reviewed and merged see 
this merge: 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-utiljvm-module/+merge/125332
 .

The changes in file: modules/ExternalModules.conf'

117 --- modules/ExternalModules.conf 2012-09-17 00:36:37 +
118 +++ modules/ExternalModules.conf 2012-09-21 15:25:26 +
119 @@ -39,11 +39,12 @@
123 +read-pdf bzr lp:zorba/read-pdf-module
128 -util-jvm bzr lp:zorba/util-jvm-module zorba-2.6
129 +util-jvm bzr lp:zorba/util-jvm-module 
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-21 Thread Matthias Brantner
Review: Needs Fixing

The module works pretty decent. I was able to extract text or generate images 
for several pdfs without any problems.
There are some minor things that should be discussed and/or fixed:

- the error seems to be too general, essentially it always raises 
JAVA-EXCEPTION no matter what goes wrong (e.g. it the given input is not a 
valid pdf)

- the java stack trace seems to be sent to standard error

- Renders the each page of the PDF document as an image. = Renders each page 
of the PDF document as an image.

- the names of the private functions should also adhere to the code conventions 
renderToImages = render-to-images

- make xqdoc failes because the comments seem to contain invalid xml
/home/mbrantner/zorba/build/URI_PATH/com/zorba-xquery/www/modules/project_xqdoc.xq:142,9:
 user-defined error [err:UE004]: Error processing module zerr:ZXQD0002 -  This 
module provides funtionality to read the text from PDF documents and
 to render PDF documents to images.
 a href=http://pdfbox.apache.org;Apache PDFBox/a library is used to
 implement these functions.
 br /
 br /
 bNote:/b Since this module has a Java library dependency a JVM required
 to be installed on the system. For Windows: jvm.dll is required on the system
 path ( usually located in C:\Program Files\Java\jre6\bin\client.
 bNote:b For Debian based Linux distributions install PdfBox and FontBox
 packages: sudo apt-get install libpdfbox-java libfontbox-java
: can not parse as XML for xqdoc: loader parsing error: Opening and ending tag 
mismatch: b line 0 and root
; raised at 
/home/mbrantner/zorba/sandbox/src/runtime/errors_and_diagnostics/errors_and_diagnostics_impl.cpp:81

- adapt the year in Copyright 2006-2009 The FLWOR Foundation. in the .xq file 
(and some other files also)

- would it make sense to return one string per page in the pdf instead of one 
big string?

- remove commented out code in read-pdf.cpp

- valgrind shows tons of invalid writes. Why? Are they critical? Is there 
anything we can do?

- would it make sense to return the images in a streaming fashion (i.e. don't 
create all base64's in a vector)?

- encoding each image shouldn't be necessary and will probably we wasted effort 
because the images might be written to a file in their binary form
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Chris Hillery
Review: Needs Fixing

You changed the URIs of the test modules back to www.zorba-xquery.com, instead 
of zorba-tests.28msec.us. That will cause test failures. Make sure you've 
merged from the trunk recently.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Chris Hillery
(Other than that the changes look fine)
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Chris Hillery
You still haven't corrected the URIs of the test modules, though 
(test/rbkt/modules/CMakeLists.txt).
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Cezar Andrei
Review: Needs Fixing

Let me add the new module.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Cezar Andrei
The ref to the new module is in, please review it too: lp:zorba/read-pdf-module 
at 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-the-module .
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Cezar Andrei
Back to zorba-tests.28msec.us in test/rbkt/modules/CMakeLists.txt and mention 
in Changelog.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Chris Hillery
Review: Approve

Great, thanks.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Zorba Build Bot
Attempt to merge into lp:zorba failed due to conflicts: 

text conflict in ChangeLog
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Matthias Brantner
Review: Needs Fixing

It looks nice (without being able to testing it ;-)

- The module should follow XQuery coding conventions. For example, no camel-case
but function and variable names using dashes.

- The read-pdf:renderToImages-internal function is not documented. This will 
probably cause
the xqdoc test to fail.

- There is a conflict in the ChangeLog.

- The schema contains some commented out code = remove?

- The declaration of the $options variable for the -internal functions should 
not allow for
an optional occurrence indicator.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Cezar Andrei
I fixed all the comments with the exception of the optional occurrence 
indicator for internal functions. I tested it with empty sequence and works 
fine.

-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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


Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/fread-pdf-trunk into lp:zorba

2012-09-19 Thread Zorba Build Bot
Voting does not meet specified criteria. Required: Approve  1, Disapprove  1, 
Needs Fixing  1, Pending  1. Got: 1 Approve, 2 Needs Fixing.
-- 
https://code.launchpad.net/~zorba-coders/zorba/fread-pdf-trunk/+merge/125338
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