Skip to content
This repository has been archived by the owner on Mar 18, 2019. It is now read-only.

Unification to featurec compiler #36

Merged
merged 23 commits into from
Feb 16, 2016
Merged

Conversation

lvh
Copy link
Contributor

@lvh lvh commented Feb 8, 2016

Closes #29. Builds on top of #27.

I am a little unhappy that I had to specify :seq all over the place for core.match to be happy. I considered termito which does make these rewrite rules simpler, but I'm concerned that they might not quite end up having the features we need, so I'll finish the tickets we have now before I try to rewrite it later. (It does work on top of core.logic, which is nice, because it means we can get rid of a library.)

@lvh
Copy link
Contributor Author

lvh commented Feb 8, 2016

I wonder why Travis isn't picking this up...

@lvh
Copy link
Contributor Author

lvh commented Feb 8, 2016

I am turning it on and off again in the hope that this will make senp^H^H^H^HTravis notice me.

@lvh lvh closed this Feb 8, 2016
@lvh lvh reopened this Feb 8, 2016
@lvh
Copy link
Contributor Author

lvh commented Feb 8, 2016

😍

@codecov-io
Copy link

Current coverage is 52.98%

Merging #36 into master will increase coverage by +0.48% as of b504a23

@@            master     #36   diff @@
======================================
  Files           17      17       
  Stmts          280     285     +5
  Branches         5       6     +1
  Methods          0       0       
======================================
+ Hit            147     151     +4
- Partial          5       6     +1
  Missed         128     128       

Review entire Coverage Diff as of b504a23

Powered by Codecov. Updated on successful CI builds.

@lvh lvh removed the needs-review label Feb 8, 2016
@lvh
Copy link
Contributor Author

lvh commented Feb 8, 2016

I removed the needs-review label because this should not be reviewed before #27.

lvh added a commit to lvh/desdemona that referenced this pull request Feb 9, 2016
@lvh lvh mentioned this pull request Feb 9, 2016
(:require [clojure.core.logic :as l]))
(:require [clojure.core.logic :as l]
[clojure.core.match :as m]
[clojure.core.logic.fd :as fd]))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unused and should be kicked out.

@lvh
Copy link
Contributor Author

lvh commented Feb 9, 2016

The partial coverage is mostly a lie. core.match generates a fall-through case that generates an error, but we don't check/care about that error right now. Maybe I should just add a test that does that...

@lvh
Copy link
Contributor Author

lvh commented Feb 9, 2016

I have added that branch test in 1666f22.

@lvh
Copy link
Contributor Author

lvh commented Feb 9, 2016

I can't tell if codecov.io hasn't updated yet, or if there's another branch I've forgotten about... Either way it certainly looks like I've covered all the interesting cases...

(try
(in-ns 'desdemona.query)
(eval compiled-query)
(eval (generate-logic-query n-answers logic-query events))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the benefit of moving this here instead of in the let above that it's more lazily evaluated? That it's actually executed inside the right namespace? Or is the functionality identical and it's just for organization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is purely organization.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, thanks.

lvh added a commit that referenced this pull request Feb 16, 2016
Unification to featurec compiler
@lvh lvh merged commit afbbcc5 into RackSec:master Feb 16, 2016
@lvh lvh deleted the query-featurec-compiler branch February 16, 2016 15:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants