<< Back to previous view

[MATCH-117] Outdated dependencies Created: 02/Dec/16  Updated: 07/Dec/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: daniel sutton Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: dependencies
Environment:

Emacs 25, CIDER


Attachments: Text File 0001-Update-clj-cljs-and-remove-nrepl-dependency.patch     Text File 0001-Update-clj-cljs-and-remove-nrepl-dependency.patch    
Patch: Code
Waiting On: David Nolen

 Description   

The Clojure version specified in core is 1.6, which is lower than the minimum supported version in CIDER (1.7)
The Clojurescript version is "0.0-2496" which seems rather outdated compared to the current "1.9.293"
The lein-cljsbuild is "1.0.4-snapshot" instead of current 1.1.4

Rather critically, this specifies its own version of cider-nrepl, a very out of date 0.8.1, rather than letting CIDER inject its current version (stable 0.14 or dev 0.15). These happen automatically now depending upon the version of CIDER you are running.

this just updates these versions in the project.clj file.



 Comments   
Comment by Enzzo Cavallo [ 05/Dec/16 5:30 AM ]

Please, bump tools.analyzer.jvm too.

[org.clojure/tools.analyzer.jvm "0.6.10"]

Comment by daniel sutton [ 05/Dec/16 7:23 AM ]

Patch added with `[org.clojure/tools.analyzer.jvm "0.6.10"]`

Comment by Alex Miller [ 06/Dec/16 10:04 AM ]

Generally we try to maintain the lowest possible minimum requirement for Clojure so that the lib can be used by as wide a group of users as possible. afaict, you don't need any new features in Clojure 1.7 or 1.8 and there is no reason to update this minimum version. You can still use it with CIDER, so that seems like a non-argument.

Comment by daniel sutton [ 07/Dec/16 10:25 AM ]

I was under the impression that the lib would use whatever versions of Clojure were in the project that loaded the lib. So updating the project.clj file affected development only. If this is incorrect I'd certainly agree with you.

CIDER doesn't officially support below 1.7 and prints a repl warning stating that nrepl and CIDER might have communication issues.





[MATCH-118] Simplify computation of necessary column Created: 06/Dec/16  Updated: 06/Dec/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: daniel sutton Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: patch
Environment:

Fedora, CIDER, clojure 1.8


Attachments: Text File 0001-Increase-speed-for-necessary-column.patch    
Waiting On: David Nolen

 Description   

From match.clj:
;; IMPORTANT NOTE: this calculation is very very slow,
;; we should look at this more closely - David

This has to do with computing the necessary column, ie, the column that contains the most number of matches we will have to do. This patch simplifies this logic which is spread out over several functions and puts it into one location. Further, it improves complexity. When computing the necessity of a column, only patters above wildcards in a column need to be computed. the way the previous implementation worked was to compute the value for each entry in the pattern matrix and then check above each entry to see if there were wildcards. Now, we just group by column and then reduce on the column, adding up the same values as last time and immediately returning the accumulator with `(reduced acc)` if we hit a wildcard. This saves the repeated traversal of each column for each entry. A nice benefit is locality of everything inside of a single function.

I've timed this against master with the entries from `bench.clj` and was actually a little surprised to see that there was no time speedup, however.






[MATCH-116] core.match clojurescript should be self-host compatible Created: 23/Oct/16  Updated: 06/Dec/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Yehonathan Sharvit Assignee: David Nolen
Resolution: Unresolved Votes: 8
Labels: bootstrap

Attachments: Text File MATCH-116.patch    

 Description   

core.match clojurescript should be self-host compatible



 Comments   
Comment by Yehonathan Sharvit [ 23/Oct/16 9:20 AM ]

Basically what I did was to take `match.clj` from https://github.com/clojure/core.match/blob/master/src/main/clojure/clojure/core/match.clj

renamed in to `match.cljc` and make a branch for `:cljs`.

Patch is attached

Comment by Yehonathan Sharvit [ 23/Oct/16 9:22 AM ]

I tested it with KLIPSE - http://app.klipse.tech/?cljs_in.gist=viebel/10da8338c4a1573fab8c04989e471704&external-libs=%5Bhttps%3A%2F%2Fraw.githubusercontent.com%2Fviebel%2Fcore.match%2Fmaster%2Fsrc%2Fmain%2Fclojure%2F%5D

Comment by Alex Miller [ 27/Oct/16 8:52 AM ]

Please note that at the moment our automated CI build box is unable to build or release projects containing cljc files. I am actively working on fixing that but until it is, this can't move forward. (And after it's fixed, I would be happy to do so.)

Comment by Yehonathan Sharvit [ 27/Oct/16 8:55 AM ]

Anyway, I'd be happy to get feedback on my patch.
It's a lot of code dealing with the differences between cljs and clojure types.

Comment by Yehonathan Sharvit [ 06/Dec/16 7:06 AM ]

Now that cljc is fully supported, could someone take a look at this patch? Alex? David?

Comment by Alex Miller [ 06/Dec/16 10:18 AM ]

The other question is whether we want to require Clojure 1.7 as a dependency for core.match, which means removing a lot of older versions from support. I think I'm ok with that, but would like to hear more from community too.





Generated at Wed Dec 07 10:36:17 CST 2016 using JIRA 4.4#649-r158309.