The intro is great!
There are examples missing for each function description.
Also I would like to understand the best practice for documenting XML
parameters.
Currently it's pretty confusing.
Matthias what's your take on it?
--
As far as I can tell, I've already given examples. Since you're so picky and
only you seem to know what you want, why don't you just add the documentation
you want yourself?
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed
If you give/write a good overview of the possible options for parse/serialize I
can definitely do that. Right know I have no clue of what's possible or not
which is gonna the case of our users too.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team
Why can't you just look at the json-options schema? There are only two options.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Review: Approve
The reported bugs were fixed, the documentation also looks good.
As far as William's last remark, IMO building the XQDoc documentation and
looking at the 'json-options' schema solves the problem.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Status: Needs review = Approved
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
--
https://code.launchpad.net/~zorba-coders/zorba/feature
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Commit Message changed to:
I think I've finally got it.
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
--
https://code.launchpad.net/~zorba
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Commit Message changed to:
New JSON parser and module.
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
--
https://code.launchpad.net/~zorba
Validation queue starting for merge proposal.
Log at:
http://zorbatest.lambda.nu:8080/remotequeue/feature-json_parser-2012-02-15T17-27-26.321Z/log.html
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed to branch lp:zorba.
--
The attempt to merge lp:~zorba-coders/zorba/feature-json_parser 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 feature-json_parser-2012-02-15T17-27-26.321Z
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Status: Approved = Needs review
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
--
https://code.launchpad.net/~zorba-coders/zorba/feature
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Status: Needs review = Approved
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
--
https://code.launchpad.net/~zorba-coders/zorba/feature
Validation queue starting for merge proposal.
Log at:
http://zorbatest.lambda.nu:8080/remotequeue/feature-json_parser-2012-02-16T00-25-35.097Z/log.html
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Validation queue job feature-json_parser-2012-02-16T00-25-35.097Z is finished.
The final status was:
All tests succeeded!
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Status: Approved = Merged
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
--
https://code.launchpad.net/~zorba-coders/zorba/feature
I approve the fix for bug #930573 and opened #932186 which depends on this
merge proposal.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
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 :
Review: Needs Fixing
Still pending on bug #930573
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/92900
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Zorba implements that proposed by John Snelson. - is that correct english?
I'm not sure.
For John Snelson representation example, please add the corresponding JSON and
XQuery function call above.
For the JSONML example, please add the corresponding JSON and XQuery function
call above.
I think
I have reopened bug #920717.
This merge is still pending on the resolution of this bug.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/91959
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Zorba implements that proposed by John Snelson. - is that correct english?
I'm not sure.
Yes.
In parse#1, add a small example.
Returns: said XDM instance. is that correct?
Yes.
In parse#2, add a small example.
Add a description of how to build element(json-options:options)
I would
If the documentation is correct in the source, I will fix everything else.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/91959
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Review: Needs Fixing
This merge request is pending on bug #920717
Also there are some documentation issues that have not been addressed yet.
How do you we do this?
Do you make another shot at it first or we should have call with Matthias to
discuss how to review it?
--
Do you take another shot at it first or should we have a call with Matthias to
discuss how to improve the module documentation?*
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/91959
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
If you want the documentation fixed, you need to specify exactly what needs
fixing -- I'm not a mind-reader.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/91959
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
Review: Needs Fixing
I believe that bug #920717 isn't fixed.
The intro has two examples of XML.
For each of the XML, you should add the associated JSON.
Would it be possible to get all the bug reports that Sorin created associated
to this merge request?
--
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/91959
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 :
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/91360
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 :
Review: Needs Fixing
The module works great.
There is room for improvement regarding its documentation.
There are many ways to represent JSON data in XML - There are two ways to
represent JSON data in XML:
- bullet 1 (John Snelson's format)
- bullet 2 (JSON)
Then add two examples, you can take
No, the statement There are many ways to represent JSON data in XML is
correct as it is. There *are* *many* ways: Zorba only implements *two* of
those *many* ways.
Adding *two* examples is too much. API documentation isn't supposed to be the
primary documentation.
--
Review: Needs Fixing
I think William is right. We should have a couple of examples which help the
user to get started with this module. Those could either be inline in the text
or links generated using the @example xqdoc tag.
The serialize functions should be annotated %ann:streamable because
Review: Needs Fixing
1) Should be possible to call json:serialize(json:parse(VALID_JSON))
where VALID_JSON is any valid JSON string
see failing test test/rbkt/zorba/json/json-snelson-serialize-parse
added bug lp:920717
2) array and object closed prematurely in json:parse
see failing tests
r10618 contains all the tests mentioned above.
The tests are passing because they are marked as EXPECTED_FAILURES.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89616
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
Why did you mark them as EXPECTED_FAILURE if they're not expected to fail?
Presumably, you *want* them fixed, right?
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89616
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
EXPECTED_FAILURE does *not* mean negative test. It means this is broken, we
know it's broken, and we're working on it. That's why you have to supply a bug
number to the macro. It probably should be renamed KNOWN_BUG.
--
It means known bug for things that are in the trunk. If it's on a branch (as
this is), then either (1) it will be fixed before it's merged into the trunk
(at which point the EXPECTED_FAILURE will have to be removed since it will no
longer be failing -- which begs my question of, Why put it in
Hmm... valid points. I think there's some value in using EXPECTED_FAILURE()
anyway, since it documents the relationship to new bugs that are filed.
It does introduce the possibility of unintentionally merging a new bug onto the
trunk, but the diff will clearly show a new EXPECTED_FAILURE()
It means known bug for things that are in the trunk. If it's on a branch
(as this is), then either (1) it will be fixed before it's merged into the
trunk (at which point the EXPECTED_FAILURE will have to be removed since it
will no longer be failing -- which begs my question of, Why put it in
Chris Hillery wrote: I think there's some value in using EXPECTED_FAILURE()
anyway, since it documents the relationship to new bugs that are filed.
The relationship to new bugs *from* ___?
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89616
Your team Zorba
Documents the relationship of the newly-added failing tests to the bugs
tracking those issues.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89616
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post
IMO, you didn't *need* to add any comment to the merge proposal other than
Bugs filed -- I can read the bugs myself.
Sorin wrote: Keep in mind that I have spent my time in order to give you a
hand in pointing out the issues I found.
It's all of our job to review each others' code under the New
@Chris: Hmmm... again, because this is a branch and no Zorba user will ever see
this stuff, it's at best only marginally useful.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89616
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list:
Paul, let me ask you something: are you sure you are not missing the point?
IMHO the point is to fix the issues that were raised ASAP.
I have spent my time in order to give as many details as possible in order to
fix them ASAP.
Also I have committed a fix in the branch for one of the opened
IMO, you didn't *need* to add any comment to the merge proposal other than
Bugs filed -- I can read the bugs myself.
FYI: Matthias asked me (in a separate email discussion) to add a small review
of the issues I fount in the merge proposal: please take this issue with him.
Sorin wrote: Keep in
No, the point is *not* to fix this issues ASAP. The feature is not a
high-priority feature, so there's no reason to do this ASAP. My *only* point
was why you added EXPECTED_FAILURE lines to the CMakeLists.txt file -- that's
it.
But it's moot now since I've removed all the new
No, the point is *not* to fix this issues ASAP. The feature is not a high-
priority feature, so there's no reason to do this ASAP. My *only* point was
why you added EXPECTED_FAILURE lines to the CMakeLists.txt file -- that's it.
But it's moot now since I've removed all the new
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89616
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 :
The proposal to merge lp:~zorba-coders/zorba/feature-json_parser into lp:zorba
has been updated.
Commit Message changed to:
New JSON parser and module.
Fixed the missing quote in the documentation.
For more details, see:
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge
I have resubmitted the merge proposal after committing the fix for the JSON
tests.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89008
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Review: Needs Fixing
The documentation is missing some examples.
The example section at http://docs.basex.org/wiki/JSON_Module is a good place
to get inspired.
Why A chars have a backslash in error code descriptions:
ZJSE0001 if \a $xml is not a document or element node.
ZJSE0002 if \a $xml
Doesn't seem to work for xqdoc
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/89008
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 :
Review: Needs Fixing
I get the following error when running the make doc target:
Zorba error [zerr:ZXQD0002]: Using this module, you can parse JSON data into
XML, manipulate it like any
other XML data using XQuery, and serialize the result back as JSON.
There are many ways to represent JSON
Those are the old jansson tests. I have to ask Matthias what to do about them.
--
https://code.launchpad.net/~zorba-coders/zorba/feature-json_parser/+merge/88721
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Also the make doc target returns the following error:
The make xqdoc fails because there are 3 modules in the ZorbaManifest.xml all
linked to the same URI, 'http://www.zorba-xquery.com/modules/converters/json'.
IMO, the fix would be to:
- make a separate branch where the deprecated external
54 matches
Mail list logo
