stuphlabs / pullcord

A reverse proxy that allows scaling down a cloud service to zero servers without sacrificing (eventual) availability.
GNU Affero General Public License v3.0
4 stars 2 forks source link

Closes #163 - Switching to fmt.Errorf #181

Closed proidiot closed 5 years ago

proidiot commented 5 years ago

Switching a few errors.New(fmt.Sprintf(... calls to fmt.Errorf(...

loslosbaby commented 5 years ago

Did you find all occurrences of 'unmarsheled'?

proidiot commented 5 years ago

@loslosbaby yep! I grepped for it, got all the remaining instances of "unmarshaled" and "Unmarshaled" in all the files that weren't external or uncommitted backups.

To give you a head's up: the standard library interface name encoding/json.Unmarshaler just has one "L", probably because somebody misspelled it and then fell back on the whole "name interfaces by adding -er" trope as an excuse. However, that doesn't automatically make using "unmarshaled" right, especially since we are using a different suffix, so I do think correcting it in our codebase is indeed the right move here.

loslosbaby commented 5 years ago

Right but I also see “unmarsheled”...with an “e”!!

On Thu, Dec 20, 2018 at 21:03 Charles Southerland notifications@github.com wrote:

@loslosbaby https://github.com/loslosbaby yep! I grepped for it, got all the remaining instances of "unmarshaled" and "Unmarshaled" in all the files that weren't external or uncommitted backups.

To give you a head's up: the standard library interface name encoding/json.Unmarshaler just has one "L", probably because somebody misspelled it and then fell back on the whole "name interfaces by adding -er" trope as an excuse. However, that doesn't automatically make using "unmarshaled" right, especially since we are using a different suffix, so I do think correcting it in our codebase is indeed the right move here.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/stuphlabs/pullcord/pull/181#issuecomment-449225763, or mute the thread https://github.com/notifications/unsubscribe-auth/AAay_EAMyuTd1F6Q2GsR6M3u2uWvmetxks5u7E-EgaJpZM4ZQrxj .

proidiot commented 5 years ago

D'oh! Fixed it now.

It is probably worth automatically running the misspell linter in the future.... maybe a dedicated bug report is called for here? coughcough