- Notifications
You must be signed in to change notification settings - Fork18
Uh oh!
There was an error while loading.Please reload this page.
Conversation
coveralls commentedSep 24, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Pull Request Test Coverage Report forBuild 1268223903Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 -Coveralls |
@@ -36,6 +36,7 @@ import ( | |||
const ( | |||
goosWindows = "windows" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's unfortunate that Go doesn't seem to offer these constants in their code anywhere. It looks like Go themselves use hardcoded string comparisons everywhere:https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/golang/go%24+%22windows%22&patternType=literal -- given that, maybe it's okay for us to do that too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Personally I think that doesn't sit well unless at least a type (like TS union of strings as a type alias) is defined. That feels a bit like walking on quicksand.
I don't know their reason, but I think it's reasonable we reach for them in our own domain model/application code base. I'd expect there to be some concept of a constant, enumeration or type in other languages when approaching a similar problem. Go sometimes makes odd choices IMO :P
Uh oh!
There was an error while loading.Please reload this page.
Inspired fromhttps://github.com/cdr/coder-cli/pull/446/checks?check_run_id=3695106435