Add support for repeated sequences#60
Conversation
|
Hi @Mikaayenson! It's exciting to see you dive right in to this issue. I think there are some more pieces that this is missing before it's ready to make sure all the edge cases are covered:
If you want to see an example of how those flags are handled, check out #59 or some of the other PRs that add Elasticsearch syntax handling behind the flag. I'm happy to sync up and go over anything in the code base if you need. Feel free to DM me (on ela.st/slack or zoom). |
rw-access
left a comment
There was a problem hiding this comment.
wow, nice turnaround! that was fast. 🏃♂️
just a few nits and I think a couple of tests needed, looks really good so far
|
@rw-access Thanks for the code walkthrough and feedback! I think it should be good to go now. 🤞 |
rw-access
left a comment
There was a problem hiding this comment.
LGTM, just a few small tweaks
brokensound77
left a comment
There was a problem hiding this comment.
Nice job! I left a few comments
Pending resolution to @brokensound77's comments
brokensound77
left a comment
There was a problem hiding this comment.
two small changes, then LGTM 🎉
|
Alright! Thanks @brokensound77. I think its ready to go, but ill wait until tomorrow just in case there's anything else. |
Resolves #57
Details
Updated the grammar and added sample test case to support repeated sequences.