uber-go / ratelimit

A Go blocking leaky-bucket rate limit implementation
MIT License
4.32k stars 300 forks source link

Only benchmark on the latest Go version #88

Closed rabbbit closed 2 years ago

rabbbit commented 2 years ago

See https://github.com/uber-go/ratelimit/issues/86#issuecomment-1146895304

Also:

codecov[bot] commented 2 years ago

Codecov Report

Merging #88 (e9d5b24) into main (0fa90d2) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #88   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           70        70           
=========================================
  Hits            70        70           

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 0fa90d2...e9d5b24. Read the comment docs.

storozhukBM commented 2 years ago

It failed after merge because it still runs on go version go1.17.10 linux/amd64

storozhukBM commented 2 years ago

Easy way to fix it would be to move

    @$(GOBIN)/benchstat stat.txt
    @$(GOBIN)/benchstat -csv stat.txt > stat.csv
    @$(GOBIN)/benchart 'RateLimiter;xAxisType=log' stat.csv stat.html
    @open stat.html

into a separate make target

rabbbit commented 2 years ago

Yeah, I was expecting the latest in the action to execute Go1.18. Somehow it didn't - I deferred debuging the action.

Separately make target sgtm too.