Housekeeping! #191
Housekeeping! #191
Conversation
| @@ -27,13 +27,13 @@ func SettingsPollOn(c echo.Context) error { | |||
| gaia.Cfg.Poll = true | |||
| err = pipeline.StartPoller() | |||
| if err != nil { | |||
| c.String(http.StatusBadRequest, err.Error()) | |||
| return c.String(http.StatusBadRequest, err.Error()) | |||
There was a problem hiding this comment.
@michelvocks We had a couple of these. :DDDD we weren't returning errors man. :D
Codecov Report
@@ Coverage Diff @@
## master #191 +/- ##
==========================================
- Coverage 62.59% 62.28% -0.32%
==========================================
Files 48 48
Lines 3914 3932 +18
==========================================
- Hits 2450 2449 -1
- Misses 1069 1079 +10
- Partials 395 404 +9
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Nice! Thanks @Skarlso
Wondering if we should try to check errors in general instead of explicitly ignoring them?
| @@ -41,7 +41,7 @@ type Payload struct { | |||
|
|
|||
| func signBody(secret, body []byte) []byte { | |||
| computed := hmac.New(sha1.New, secret) | |||
| computed.Write(body) | |||
| _, _ = computed.Write(body) | |||
There was a problem hiding this comment.
I'm wondering why we explicitly ignore errors here instead of doing what is recommended - checking the error and fail if there is one? 😅
| "net/http" | ||
| "strconv" | ||
| "time" | ||
|
|
||
| "github.com/gaia-pipeline/gaia/helper/stringhelper" | ||
|
|
There was a problem hiding this comment.
I don't know what is wrong with go-import (this always happens for me too) but this looks really ugly 😒
| @@ -27,13 +27,13 @@ func SettingsPollOn(c echo.Context) error { | |||
| gaia.Cfg.Poll = true | |||
| err = pipeline.StartPoller() | |||
| if err != nil { | |||
| c.String(http.StatusBadRequest, err.Error()) | |||
| return c.String(http.StatusBadRequest, err.Error()) | |||
| @@ -18,6 +17,8 @@ import ( | |||
| "testing" | |||
| "time" | |||
|
|
|||
| "github.com/pkg/errors" | |||
|
|
|||
|
@michelvocks I have the handling in a different PR. This is part 1 basically. The handling and propagations of some of these errors when I was not ignoring them was actually causing some test failures. And thus require a lot more thought and work to have them right. So I wanted to do this in one patch and then go over all the |
|
Gotcha! Cool. Then LGTM 😄 |
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #191 +/- ##
==========================================
- Coverage 62.59% 62.28% -0.32%
==========================================
Files 48 48
Lines 3914 3932 +18
==========================================
- Hits 2450 2449 -1
- Misses 1069 1079 +10
- Partials 395 404 +9 ☔ View full report in Codecov by Sentry. |
@michelvocks You are going to love this one... :D