travis-ci/travis-worker — comment on commit dd066256825365e1a3dd5602f62c4016c0288109
File:  line 0
By svenfuchs:

I'm not sure what other milestones you have on the plate, but I'd like to see the following. Not saying that *you* need to implement that, but at some point I think things should be refactored into that direction:

* The worker class now has to deal with lots of AMQP specific logic. I don't think it should. Can we extract that to some kind of Connection class or something similar?

* Can we rename BuildLogStreamer and StateReporter to a common Reporter namespace? Like Reporter::State and Reporter::BuildStreamer or similar.

* Please do not change the formatting styles. Methods below protected/private should be indented. That also makes it harder to follow diffs.

* Can we please replace instance vars like @connection with an initializing connect method with either a) an attr_reader and pass it to initialize or b) a lazy accessor method (def connection; @connection ||= ...; end)?