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

Support matching on variables that are collections #163

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

felipefzdz
Copy link
Contributor

@felipefzdz felipefzdz commented Jun 8, 2016

This PR addresses #123 issue.

I was TDDing this feature outside-in and I've figured out that production code was already able to handle collections for multi prime matching. I'd like to keep the tests though as they'll serve as documentation and to avoid regressions.

I've updated the documentation with an example about using collections when matching variables.

Let me know if you're happy with the PR and I'll squash the commits.

Meanwhile reading the code I've found a couple of TODOs to make the code more robust and maintainable, e.g.

  • validation error if variable types length != outcome variable matcher length
  • validate PrimePreparedMulti
  • deal with a prime that doesn't exist
  • maybe say the last outcome is the default or make a default mandatory?
  • generalise all the prepared stores, very little difference

I'll address those in a future PR, if that's ok.

@felipefzdz felipefzdz force-pushed the issue_123 branch 4 times, most recently from 6aeddd7 to 2431c9d Compare June 8, 2016 14:13
@chbatey
Copy link
Member

chbatey commented Jul 13, 2016

Somehow had turned off my notifications for scassandra :( sorry @felipefzdz looking now

@felipefzdz
Copy link
Contributor Author

No worries :)

@chbatey
Copy link
Member

chbatey commented Sep 5, 2016

Sorry been out out of the (c|s)assandra world lately. Back on this week so will review this.

@tolbertam
Copy link
Contributor

tolbertam commented Jan 11, 2017

D'oh, just realized my recent PR created some conflicts here, however I think that PR added support matching of collections, but i'll have to test it out. I'll use the test cases here to see if that was the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants