Upgrade clojure version #25

Closed
nickmbailey opened this Issue Apr 20, 2012 · 5 comments

2 participants

@nickmbailey
Collaborator

We are on 1.2.1 right now. The tests right now break on 1.3. Almost certainly due to the auto boxing behavior changes in 1.3. There might be some issues with any dynamic vars we have as well.

Also 1.4 is out, which again changes boxing behavior. Ints will box to Long in 1.3 but integer in 1.4 believe. We should probably just upgrade straight to 1.4.

@pingles
Owner

Sounds like a plan- have some time tomorrow so might give it a go, as you say- looks like just some numeric behaviour has changed.

@pingles
Owner

So, @nickmbailey I've done some investigating- I think the problem is down to the numeric handling. Specifically, it looks like when Hector serialises the arguments the IntegerSerializer throws a class cast exception.

I'm in 2 minds with what to do:

1) We change the tests so that we specify long/type inferring serialisation.
2) We would need to wrap the serialisation to ensure that the arguments were explicitly converted before being pumped into the serializer.

I'm leaning towards #1- it means less funky code (we're not doing any additional wrapping etc.) but it means that anyone serialising as integers in 1.2 is likely to have class cast exceptions when trying to store values without explicitly converting themselves.

Thoughts?

In case #1 is good I've pushed changes onto a branch (with commit 3ac4548) ready to be merged back to master :)

@nickmbailey
Collaborator

1 seems fine to me at least for now. I mean doesn't 1.2 have the same problem except in reverse? If we used long serializer it would get passed ints and break?

Some custom wrapper for serializing things before they get to hector would be nice at some point but I don't see any reason to add it now.

@pingles
Owner

I've merged the test changes into master and pushed a 0.2.1 release to Clojars- we're now building against Clojure 1.4.0.

@pingles pingles closed this Jun 9, 2012
@nickmbailey
Collaborator

@pingles, sorry for the late reply. I've been out of the country for two weeks. The changes from option 1 should be fine for now I think. I'll probably make a ticket for either adding documentation about integers so people aren't confused or maybe looking into something for option 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment