Hi Riyafa,
the change looks good, I've added just one comment.
On the question of jdm:keys vs. js:keys the specification says in [4]:
The XQuery data model uses accessors to explain the data model.
Accessors are
not exposed to the user and are only used for convenience in this
specification. Objects have the following accessors: jdm:keys [...]
So jdm:keys is not supposed to be accessible to the user. Now I think
there
are 2 things that we could do:
1) have both the jdm:keys accessor and the jn:keys function and
implement
jn:keys by using jdm:keys or
2) just implement everything inside of the jn:keys evaluator.
Does this make sense?
Cheers,
Till
[4]
http://jsoniq.org/docs/JSONiqExtensionToXQuery/html-single/index.html#idm139680641300880
On 2 Jul 2016, at 1:24, Riyafa Abdul Hameed wrote:
Hi,
I have created a PR with the suggested changes[1] except for the 3rd
suggestions. The implementation of jdm:keys is based on [2] and
jn:keys is
based on [3].
[1] https://github.com/apache/vxquery/pull/80
[2]
http://jsoniq.org/docs/JSONiqExtensionToXQuery/html-single/index.html#idm139680641300880
[3]
http://jsoniq.org/docs/JSONiqExtensionToXQuery/html-single/index.html#idm139680622840960
Thank you.
Yours sincerely,
Riyafa
On 2 July 2016 at 00:33, Till Westmann <[email protected]> wrote:
Hi,
it’s really great to see the object navigation. This is really nice
progress!
Looking at the commits I saw a few things that don’t seem to be
quite
right.
Could you take a look if those can be fixed?
1) The return type for jn:null is specified to be xs:string*, that
seems
wrong.
2) The return type jdm:keys is specified to be item(), that seems
wrong as
well.
3) There’s a lot of overlap between jdm:keys and jn:keys. Do we
actually
need jdm:keys?
4) A number of the new tests don’t have a new-line at the end of
the file.
Could we add those?
Cheers,
Till
--
Riyafa Abdul Hameed
Undergraduate, University of Moratuwa
Email: [email protected]
Website: https://riyafa.wordpress.com/ <http://riyafa.wordpress.com/>
<http://facebook.com/riyafa.ahf> <http://lk.linkedin.com/in/riyafa>
<http://twitter.com/Riyafa1>
