Skip to content

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