search

hotsh / rstat.us

Simple microblogging network based on the ostatus protocol.
http://rstat.us/
Other
722 stars 215 forks source link

Elasticsearch #678

Closed carols10cents closed 12 years ago

carols10cents commented 12 years ago

Hoookay. I think this finally gets elasticsearch all happy, but doesn't require developers or node operators to use it.

Live demo available for a limited time only at https://rstatus-staging.herokuapp.com/

This includes @bglusman's PR #455.

carols10cents commented 12 years ago

I bet @wilkie is going to have some opinions on this-- I'd love for other people to have a look-see too, but I'd rather not have this merged until @wilkie gives his :+1:.

wilkie commented 12 years ago

You've satisfied most of my criteria that relate to maintainability. All of the environmental stuff that affect tests are within helpers, not the tests themselves which is good. We have a test task that ignores optional things, and test:all for travis and major-feature developers to use. Sweet. I just have the one comment about code creeping into the model specifically for tests... but otherwise it looks good.

We'll figure out this 'enhancement' strategy so we don't start having HUGE blocks of if ENV['foo'] || ENV['bar'] etc etc creeping into the code. I expect that the next thing we add like this will force some refactoring.

wilkie commented 12 years ago

Oh you did do that. Nevermind. :) I look at both the commit diffs and the total diff to double check. I didn't see the commit that undid it, oops. You could easily have removed that commit with a rebase, so I wasn't expecting it either.

Sweet. In that case :+1:

carols10cents commented 12 years ago

Yeah, I'm often torn between squashing things and preserving history of how I came to a solution.... it's often a combination, there's definitely stuff that did get squashed out of this.

Thank you for reviewing!!