Skip to content

Clojurescript support for clout - #14

Closed
roman wants to merge 14 commits into
weavejester:masterfrom
BirdseyeSoftware:cljs
Closed

Clojurescript support for clout#14
roman wants to merge 14 commits into
weavejester:masterfrom
BirdseyeSoftware:cljs

Conversation

@roman

@roman roman commented Mar 13, 2013

Copy link
Copy Markdown

Hello James,

I know this is a project that you haven't code in a while, but I took the liberty to polish it a bit and add Clojurescript support for it. Also I bumped the Clojure version to support both 1.4 and 1.5.

Both the code and the testsuite work in Clojurescript (nodejs, chrome, safari, ff, phantomjs), with a TravisCI configuration. We haven't tested thoroughly the code though, we are planning to use it in the future in some of our projects for routing apps in the front-end (Backbone style).

Let me know what you think.

Cheers.

Roman.-

@sritchie

Copy link
Copy Markdown

Any updates?

@weavejester

Copy link
Copy Markdown
Owner

Hm, for some reason these pull requests didn't show up on my feeds when they were first pushed.

I'm not certain whether or not to merge. It adds a lot of additional code. Strangely, #13 seems to be cleaner than #14. I wonder why there's two PRs to begin with.

@sritchie

Copy link
Copy Markdown

Haha, yes, and by two separate guys named Roman around the same time. Very strange.

As with these guys, I'm interested in a good, compojure compatible routing story for CLJS. #13 does look better, as that roman also upgraded ring-mock:

weavejester/ring-mock#4

Happy to help code review if you're into this in theory.

@weavejester

Copy link
Copy Markdown
Owner

Ah, they're by two different people. That makes more sense now.

Why do you need to match routes in CLJS?

@sritchie

Copy link
Copy Markdown

There may be a better way to do it, but I want to keep all of my cljs code in one big generated file (for caching purposes) and then restrict various callback registrations to certain pages. Sort of like

https://github.com/gf3/secretary

@weavejester

Copy link
Copy Markdown
Owner

Closing this PR as it's incompatible with Clout 2.0.

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