Skip to content

added transcoder for serializable objects#39

Merged
rgruener merged 1 commit intospotify:masterfrom
rgruener:master
Feb 23, 2015
Merged

added transcoder for serializable objects#39
rgruener merged 1 commit intospotify:masterfrom
rgruener:master

Conversation

@rgruener
Copy link
Copy Markdown
Contributor

Caching serializable objects is an extremely common use case and is the easiest way to use memcached and folsom out of the box.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.05%) to 92.46% when pulling 1aaed90 on rgruener:master into a88fe30 on spotify:master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.05%) to 92.46% when pulling 1aaed90 on rgruener:master into a88fe30 on spotify:master.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand the last part of this method name - typo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops sorry, definitely a typo

@spkrka
Copy link
Copy Markdown
Member

spkrka commented Feb 23, 2015

I like the idea in general, just needs some fixes before we can merge.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

missing serialVersionUID (super minor)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch! Though I guess it doesn't matter since we don't need a consistent serial id for a very short lived test class/object.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.21%) to 92.49% when pulling 9445f98 on rgruener:master into 58f9892 on spotify:master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.03%) to 92.67% when pulling 9445f98 on rgruener:master into 58f9892 on spotify:master.

@rgruener
Copy link
Copy Markdown
Contributor Author

Made all the suggested changes. Im not sure why the ci is failing as I didnt touch that code.

@spkrka
Copy link
Copy Markdown
Member

spkrka commented Feb 23, 2015

The tests were not very robust - I think I fixed that in master though. So let's just ignore the report here.

Everything looks good to me! Feel free to merge.

@rgruener
Copy link
Copy Markdown
Contributor Author

I dont think I was ever added to the Spotify organization so I do not have permissions to merge.

@nresare
Copy link
Copy Markdown
Contributor

nresare commented Feb 23, 2015

A good thing I'm lurking here, then. You should have an invite.

rgruener added a commit that referenced this pull request Feb 23, 2015
added transcoder for serializable objects
@rgruener rgruener merged commit 5d2a426 into spotify:master Feb 23, 2015
@gilles
Copy link
Copy Markdown

gilles commented Mar 5, 2015

Do you have an ETA for a release with that feature?

@spkrka
Copy link
Copy Markdown
Member

spkrka commented Mar 5, 2015

Not really, but if you just want this feature you can easily add it manually in your project for now, it's a very small amount of code (and you can look at this PR for inspiration).

@gilles
Copy link
Copy Markdown

gilles commented Mar 5, 2015

True :)

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.

5 participants