Add UUID provided function #253

fuchsst · 2022-03-30T13:20:12Z

added uuid build-in function

tests are only present for call with both parameter, as the other calls don't return a deterministic value and the QueryTests expect a constant as expected value - tested manually (worked for all three constellations with none, one and two parameter), let me know if i should add an extra test, that validates against a regex
MR is based on MR Update dependency versions #251 & Remove unused imports #252
solves Issue Generate UUID #203

larsga · 2022-04-06T11:22:58Z

let me know if i should add an extra test, that validates against a regex

Yes, please. I think that would be very valuable. It would at least prove that the code produces something that is syntactically correct.

You could also generate two UUIDs and verify that they are different from each other.

core/src/main/java/com/schibsted/spt/data/jslt/impl/BuiltinFunctions.java

core/src/test/resources/function-tests.json

functions.md

larsga · 2022-04-06T11:30:58Z

Overall this looks good. My comments are just nitpicks, really. Great work! 👍

larsga · 2022-04-12T08:50:28Z

I think this looks good now. At least I don't know of any actual problems preventing a merge.

Could you rebase it on master so I can verify that everything is OK and merge it without including the other PR?

fuchsst · 2022-04-12T17:08:08Z

I think this looks good now. At least I don't know of any actual problems preventing a merge.

Could you rebase it on master so I can verify that everything is OK and merge it without including the other PR?

done...now it's just the added new function...looking forward to see the new version on maven central :-)

larsga

This all looks great now. Thank you very much for doing this, and for adapting the design so quickly based on feedback. Several times you changed your proposal exactly the way I was thinking you should before I had time to suggest it. 👍