make it easier to move to clojure 1.3 #1

Closed
wants to merge 17 commits into
from

Projects

None yet

2 participants

@pyr
pyr commented Sep 19, 2011

by removing dependencies on clojure.contrib material

Pierre-Yves ... added some commits Sep 19, 2011
@pyr
pyr commented Sep 19, 2011

The round two commit is a bit more debateable, since it doesn't necessarily help as far as readability is concerned, but defnk didn't make it in clojure.core.incubator, so it will be left behind in clojure 1.3 it seems

@pyr
pyr commented Sep 19, 2011

With this last batch of commits, clojure contrib should be eliminated from storm's dependencies. I only have the simple topologies to play with right now. If more complex use cases don't generate any odd behavior, contrib can be removed from dependencies and people will be able to start playing with 1.3

@nathanmarz
Owner

Making Storm Clojure 1.3 compatible is definitely a good thing to do. A few notes on these commits:

  1. Like Sam said, some and find-first are not equivalent. We can just copy the find-first implementation in backtype.storm.utils to get rid of the dependency.
  2. Likewise, instead of dirtying up the code, how about we just copy defnk into backtype.storm.utils?

Copying those functions isn't great, but unless there's a Clojure 1.3 compatible dependency we can pull in, I think it's the best we can do.

Pierre-Yves ... added some commits Sep 20, 2011
Pierre-Yves Ritschard update wrong usage of some 7b47947
Pierre-Yves Ritschard typos 9ddcee7
@pyr
pyr commented Sep 20, 2011

Instead of copying find-first i used (first (filter)) which uses the same result. I usually use some with fn's returning the element which yields the same result, sorry.

I fixed the defnk uses, I just feel there's not that much to warrant copying the function over, but if you feel it's too ugly i can copy it.

@nathanmarz
Owner

Yes, I'd like to keep the usage of defnk. Let's just copy that into backtype.storm.util. To keep the history clean, I think it would be best to make a new pull request with one commit for the defnk change, one commit the the find-first change, and then other commits for the other minor changes you made. I'd do it myself, but I'd like you to get credit for the commits!

@pyr
pyr commented Sep 26, 2011

OK, will do that

On Mon, Sep 26, 2011 at 9:33 AM, Nathan Marz
reply@reply.github.com
wrote:

Yes, I'd like to keep the usage of defnk. Let's just copy that into backtype.storm.util. To keep the history clean, I think it would be best to make a new pull request with one commit for the defnk change, one commit the the find-first change, and then other commits for the other minor changes you made. I'd do it myself, but I'd like you to get credit for the commits!

Reply to this email directly or view it on GitHub:
#1 (comment)

pyr added some commits Sep 30, 2011
@pyr pyr bring back defnk f44cc73
@pyr pyr Merge branch 'master' of https://github.com/nathanmarz/storm
Conflicts:
	src/clj/backtype/storm/testing.clj
	src/clj/backtype/storm/thrift.clj
	src/clj/zilch/virtual_port.clj
0254c45
@pyr pyr make it easier to move to clojure 1.3 a2ca801
@pyr
pyr commented Sep 30, 2011

Done, I'll just need to resort the history now

pyr added some commits Sep 30, 2011
@pyr pyr re-remove contrib references 78f89fb
@pyr pyr find-first and contrib fixes abc260c
@pyr pyr Merge branch 'master' of github.com:pyr/storm into HEAD
Conflicts:
	src/clj/backtype/storm/daemon/common.clj
b672f5d
@pyr pyr make it easier to move to clojure 1.3
re-remove contrib references

find-first and contrib fixes

make it easier to move to clojure 1.3

Round two of contrib removal, replace defnk

round three find-first can also be called some

round 4: join is available in clojure.string

remove ceil dependency as code is commented out

remove dissoc-in since it is not used anymore

update wrong usage of some

typos

bring back defnk
ac95789
@pyr pyr Merge branch 'master' of github.com:pyr/storm
Conflicts:
	src/clj/backtype/storm/util.clj
df82aad
@pyr pyr a conflict slip through 7984112
@pyr pyr closed this Sep 30, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment