Closed hagane closed 8 years ago
Well, currently I'd say that it's okay in principle. You can keep that way of implementation.
All right, that should do it. Sadly, I have no idea how to automate twitter endpoint testing.
You should add a room configuration key to enable/disable Twitter plugin at least.
Ideally it would be a configuration for every room to connect to a chosen Twitter account, but currently we don't need that. Although, we already have Horta installed to multiple rooms, and only one of them need this Twitter support.
You're right. I'll see what I can do about all that.
All right, i've got it done. I'd like to add some tests for all this first and perform some acceptance tests myself.
Sadly, horta does not starts for me anymore failing with Configuration not found at 'None'.
Looks like it has no idea where to look for it's configuration and the default of horta.properties
is not so default anymore. Can anyone verify that? Maybe it's just my working copy of the code got mucked with whatever. I'll try to reclone it meanwhile.
UPD: Aye, it's broken. I'll look into it. Feel free to ignore this PR for now
Apparently, I'm daft. TwitterEndpoint was initialized before the configuration. We seriously have to have something to prevent these issues in the future.
After my caffeine high wears off and I will become lucid again, I'll add some test cases for these improvements.
I've reviewed the code. Required feature set is already covered by tests. I must have been to high on stims back then. I'll do some manual testing next evening and report back.
There, I'm satisfied. Are you satisfied, Mr. @ForNeVeR ?
Well, let's try that!
Got some serious progress. Figured somebody better confirm that this code is sane before I delve into thread pools and twitter's JSON.