go-siris / siris

DEPRECATED: The community driven fork of Iris. The fastest web framework for Golang!
Other
142 stars 16 forks source link

Fix/race condition #44

Closed Dexus closed 7 years ago

Dexus commented 7 years ago

I need at last three reviews, thank you.

To merge and fix #39

godofdream commented 7 years ago

Why do you replace the logger? logrus is nice but definitly not the fastest logger. I'm using e.g. zap.

Dexus commented 7 years ago

I have took the changes from Iris/Ion. I did not know zap, but that would be a very good alternative.

godofdream commented 7 years ago

logrus is nice but definitly not fast, see some benchmarks: https://github.com/uber-go/zap

Dexus commented 7 years ago

Okay, then I'll remove logrus and add Zap.

Any hints for zap, as i use it the first time?

codecov[bot] commented 7 years ago

Codecov Report

Merging #44 into devel will increase coverage by 4.93%. The diff coverage is 49.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel      #44      +/-   ##
==========================================
+ Coverage   47.74%   52.67%   +4.93%     
==========================================
  Files          22       18       -4     
  Lines        1240     1232       -8     
==========================================
+ Hits          592      649      +57     
+ Misses        616      542      -74     
- Partials       32       41       +9
Impacted Files Coverage Δ
core/host/proxy.go 73.8% <ø> (ø) :arrow_up:
core/nettools/server.go 0% <0%> (ø) :arrow_up:
core/nettools/addr.go 68.85% <100%> (+42.12%) :arrow_up:
core/errors/reporter.go 13.51% <13.51%> (ø)
core/errors/errors.go 46.77% <50%> (-9.64%) :arrow_down:
core/host/task.go 57.89% <62.5%> (+13.99%) :arrow_up:
core/host/supervisor.go 63.77% <66.66%> (+7.38%) :arrow_up:
core/host/world.go 75% <75%> (ø)
... and 2 more

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 d57779a...91ddbc2. Read the comment docs.

codecov-io commented 7 years ago

Codecov Report

Merging #44 into devel will increase coverage by 7.93%. The diff coverage is 52.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel      #44      +/-   ##
==========================================
+ Coverage   47.74%   55.68%   +7.93%     
==========================================
  Files          22       18       -4     
  Lines        1240     1232       -8     
==========================================
+ Hits          592      686      +94     
+ Misses        616      507     -109     
- Partials       32       39       +7
Impacted Files Coverage Δ
core/host/proxy.go 73.8% <ø> (ø) :arrow_up:
core/nettools/server.go 0% <0%> (ø) :arrow_up:
core/nettools/addr.go 80.32% <100%> (+53.6%) :arrow_up:
core/errors/reporter.go 13.51% <13.51%> (ø)
core/host/task.go 57.89% <62.5%> (+13.99%) :arrow_up:
core/host/supervisor.go 63.77% <66.66%> (+7.38%) :arrow_up:
core/errors/errors.go 83.87% <73.68%> (+27.46%) :arrow_up:
core/host/world.go 75% <75%> (ø)
... and 3 more

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 d57779a...1fc1d8c. Read the comment docs.

Dexus commented 7 years ago

@godofdream can you now again review? Zap is added and the "newline" is also added... but it should never used only when created a new error from /core/errors with any Append function on it.