Review: Approve
LGTM
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/183640
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 :
Thanks Nicolae!
The following does not seem to work:
jsoniq version 1.0;
variable $a := [];
insert (1, 2) into $a at position 1;
$a
It might be because insert(1, 2) is parsed as a function call. Would it be
possible to resolve the conflict using the lookahead (whether into is here or
not)?
Ghislain, I've fixed the null/true/false etc and select issues (points 5 and
8).
--
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/183640
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/new-jsoniq/+merge/183640
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
Hi Nicolae,
Looks good, thanks ;-) It's getting closer and closer to the git EBNFs!
Here are some comments:
1. Parser.y, line 2259: is || (block != NULL block-isEmpty()) really
needed? Will block not always be null if a BlockExpr is {} (see
StatementsAndOptionalExpr
1. Parser.y, line 2259: is || (block != NULL block-isEmpty()) really
needed? Will block not always be null if a BlockExpr is {} (see
StatementsAndOptionalExpr nonterminal)?
Nope, you can have a nested empty block {{}}.
2. Why is the OBJECT token treated specially and not mentioned in
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/168471
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 :
Attempt to merge into lp:zorba failed due to conflicts:
text conflict in ChangeLog
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/168471
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/new-jsoniq/+merge/168471
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:281
(message):
Validation queue job new-jsoniq-2013-06-10T15-23-50.144Z is finished. The
final
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 1 Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/168471
Your team Zorba Coders is subscribed to branch
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:281
(message):
Validation queue job new-jsoniq-2013-06-07T08-07-50.452Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:281
(message):
Validation queue job new-jsoniq-2013-06-06T18-12-45.616Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:281
(message):
Validation queue job new-jsoniq-2013-06-06T19-02-00.114Z is finished. The
final
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/167824
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Why would you need a qualified value if the option itself is already
qualified?
I guess because the option name and the option value may not live in the
same
namespace.
Why not? It's not as if you do schema validation on the options and their
values. The option needs to be
Review: Needs Fixing
I can not see any warning if a version declaration is present. Was it fixed? Am
I doing something wrong?
jsoniq version 1.0;
declare namespace op = http://www.zorba-xquery.com/options/features;;
declare option op:enable common-language;
{ foo : bar }
- no warning for me.
Ghislain - it seems to work if you specify the option on the command line:
% zorba --option
'{http://www.zorba-xquery.com/options/features}enable=common-language' -q
'jsoniq version 1.0; { foo : bar }'
.:1,2: Zorba static warning [zwarn:ZWST0009]: feature not supported by the
common language
Review: Approve
Approving because the last open issue is not release-critical.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Your comment makes a lot of sense too, thanks!
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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/new-jsoniq/+merge/162375
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 :
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 2
Approve, 1 Needs Fixing.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch
Thanks Nicolae, this is perfect now.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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
The commit could add something to the ChangeLog. Otherwise I'm also happy.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
I've added a mention in the ChangeLog
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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/new-jsoniq/+merge/162375
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 :
Pending approval and merge of null-type-fix before sending to the trunk.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to :
Done
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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 parser must be regenerated I think.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-05-15T16-11-04.9Z is finished. The
final
Yeah, I just fixed that unit test as well...
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-05-13T11-36-28.819Z is finished. The
final
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 2 Needs Fixing.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch
Matthias, your suggestions for two improved warnings do not change the
messages: you just switched the order of the words. The warnings are built
around the template:
feature not supported by the common language grammar{: $1}
So it is not easy to make this swap. I could probably do it,
Why would you need a qualified value if the option itself is already
qualified?
I guess because the option name and the option value may not live in the same
namespace.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch
Why would you need a qualified value if the option itself is already
qualified?
I guess because the option name and the option value may not live in the same
namespace.
Why not? It's not as if you do schema validation on the options and their
values. The option needs to be qualified to
Review: Needs Fixing
Thanks for fixing the warnings. I successfully checked, except for two things:
1.
\ (XQuery parser) or (JSONiq parser) leads to a warning about disallowed
character entity references/JSON character escape sequence. I think the warning
could be made more general and say
(by the way, character entity references do not exist in XML. There are
character references (#...;) and entity references (...;), but not both at
the same time if I am correct :-) )
Indeed you're right. I trusted Wikipedia which uses the term character entity
references and says ``The XML
Fixed all the raised issues.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
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
It looks good on the whole. For convenience, I am pasting the list of the
findings I sent to the zorba-dev mailing list, as of revision 11342 of this
branch, here:
- The dot context item should be a syntax error in the JSONiq parser, but {
foo : bar } ! . seems to parse
Review: Needs Fixing
1. improve warning messages:
feature not supported by the common language grammar: JSONiq dot object lookup
=
jsoniq dot object lookup not supported by common language
feature not supported by the common language grammar: context item expression;
use
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-05-03T14-40-57.855Z is finished. The
final
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/162375
Your team Zorba Coders is subscribed to branch lp:zorba.
--
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-05-01T17-24-52.433Z is finished. The
final
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159739
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/new-jsoniq/+merge/159739
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-04-19T20-39-52.046Z is finished. The
final
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159739
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-04-19T21-18-54.417Z is finished. The
final
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159739
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/new-jsoniq/+merge/159739
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/new-jsoniq/+merge/159736
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 :
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 1 Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159736
Your team Zorba Coders is subscribed to branch
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159739
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-04-18T22-51-54.931Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-04-18T23-01-21.401Z is finished. The
final
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 1 Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159739
Your team Zorba Coders is subscribed to branch
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-04-17T18-23-02.191Z is finished. The
final
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159477
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 :
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 1 Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159477
Your team Zorba Coders is subscribed to branch
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 1 Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159477
Your team Zorba Coders is subscribed to branch
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/159477
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 :
Attempt to merge into lp:zorba failed due to conflicts:
text conflict in src/compiler/parser/jsoniq_parser.cpp
text conflict in src/compiler/parser/jsoniq_parser.hpp
text conflict in src/compiler/parser/jsoniq_scanner.cpp
text conflict in src/compiler/parser/xquery_parser.cpp
text conflict in
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
Traceback (most recent call last):
File /home/ceej/zo/testing/zorbatest/tester/proposal_comment.py, line 36,
in module
sys.exit(main(sys.argv))
File
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-04-17T01-24-44.705Z is finished. The
final
Review: Approve
Looks good. I have added a test. I approve but I don't understand why the
generated xquery parser and scanner files are updated in this merge proposal.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/157559
Your team Zorba Coders is subscribed to branch
I'm not sure either. It might be that the scanner .cpp file in the trunk had
been generated with a different version of flex so cmake decided to rebuild it.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/157559
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/157559
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 :
Voting does not meet specified criteria. Required: Approve 1, Disapprove 1,
Needs Fixing 1, Pending 1, Needs Information 1, Resubmit 1. Got: 1
Approve, 1 Pending.
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/157559
Your team Zorba Coders is subscribed to branch
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/155270
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-03-26T07-34-49.835Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-03-27T00-36-24.69Z is finished. The
final
Review: Needs Fixing
I tried the following query:
jsoniq version 1.0; \u0041
IMHO, it should return a but it returns \u0041 instead. I think the parsing
is done right but the parser also needs to replace the escapes, right?
--
Review: Approve
--
https://code.launchpad.net/~zorba-coders/zorba/new-jsoniq/+merge/155270
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 attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-03-26T02-28-47.758Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-03-23T01-18-07.286Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-03-22T00-59-31.055Z is finished. The
final
The attempt to merge lp:~zorba-coders/zorba/new-jsoniq into lp:zorba failed.
Below is the output from the failed tests.
CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:275
(message):
Validation queue job new-jsoniq-2013-03-22T02-07-22.831Z is finished. The
final
79 matches
Mail list logo
