Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/add missing fn trace #3597

Merged
merged 4 commits into from
Nov 3, 2020

Conversation

dizzzz
Copy link
Member

@dizzzz dizzzz commented Oct 30, 2020

Adding fn:trace#1 function
Testcases are included

@dizzzz dizzzz requested review from joewiz, adamretter, wolfgangmm and a team October 30, 2020 20:09
@@ -38,40 +39,53 @@
*/
public class FunTrace extends BasicFunction {

public final static FunctionSignature signature =
public final static FunctionSignature[] signatures = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any interest in using the newer FunctionDSL approach? It has some advantages...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was in doubt to apply that pattern, but was not sure how to start....

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes lets do that, do you have a good sample for it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the CacheModule isn't a bad example.

Copy link
Member

@adamretter adamretter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment.

@adamretter adamretter added the enhancement new features, suggestions, etc. label Nov 1, 2020
@adamretter adamretter added this to the eXist-5.3.0 milestone Nov 1, 2020
Copy link
Member

@joewiz joewiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dizzzz Thank you for helping eXist advance toward full coverage of the XQuery 3.1 spec! I ran this PR against https://github.com/w3c/qt3tests/blob/master/fn/trace.xml, using https://github.com/eXist-db/exist-xqts-runner (commands: sbt clean compile, sbt assembly, and target/scala-2.13/exist-xqts-runner-assembly-1.0.0.jar -ts fn-trace). With this PR, eXist passes 24 of the 27 tests for fn:trace (see fn-trace-junit-data-PR-3597.zip). Surprisingly, develop passes 25 of the 27 tests (see fn-trace-junit-data-develop.zip). However, scanning through the failures, it seems to me that none of them appear to be consequential.

@joewiz joewiz added the xquery issue is related to xquery implementation label Nov 1, 2020
@dizzzz
Copy link
Member Author

dizzzz commented Nov 2, 2020

image

@dizzzz
Copy link
Member Author

dizzzz commented Nov 2, 2020

updated description @adamretter @joewiz

Copy link
Member

@adamretter adamretter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dizzzz Nicely done :-) How did you find the FunctionDSL?

@dizzzz
Copy link
Member Author

dizzzz commented Nov 3, 2020

It gives more structure than the original WoW I guess. I will start using it from now :-)

@adamretter
Copy link
Member

@dizzzz What does "WoW" stand for?

I also just pushed a very small improvement ;-)

@dizzzz
Copy link
Member Author

dizzzz commented Nov 3, 2020

Way Of Working :-P

Copy link
Member Author

@dizzzz dizzzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dizzzz dizzzz merged commit 7f1b216 into eXist-db:develop Nov 3, 2020
@dizzzz dizzzz deleted the feature/add_missing_fn_trace branch November 3, 2020 15:34
joewiz added a commit to eXist-db/documentation that referenced this pull request Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new features, suggestions, etc. xquery issue is related to xquery implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants