tumblr / colossus

I/O and Microservice library for Scala
Apache License 2.0
1.14k stars 96 forks source link

Worker manager refactor #629

Closed benblack86 closed 6 years ago

benblack86 commented 6 years ago

Main aim was just to improve the logging coming from the worker manager by adding the io system name to each log statement. Whilst I was there I also:

No logic changes! Fixes #619

@aliyakamercan @jlbelmonte @amotamed @dxuhuang @DanSimon

codecov-io commented 6 years ago

Codecov Report

Merging #629 into develop will decrease coverage by 0.01%. The diff coverage is 82.92%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #629      +/-   ##
===========================================
- Coverage    84.76%   84.75%   -0.02%     
===========================================
  Files           98       99       +1     
  Lines         4432     4435       +3     
  Branches       366      354      -12     
===========================================
+ Hits          3757     3759       +2     
- Misses         675      676       +1
Impacted Files Coverage Δ
...lossus/src/main/scala/colossus/core/IOSystem.scala 93.75% <ø> (ø) :arrow_up:
...s/src/main/scala/colossus/core/WorkerFactory.scala 100% <100%> (ø)
...s/src/main/scala/colossus/core/WorkerManager.scala 86.45% <80.55%> (-1.3%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e1bb232...ad3cc51. Read the comment docs.

benblack86 commented 6 years ago

@amotamed I believe both WorkerFactory and DefaultWorkerFactory should only be used internally, therefore kept private.