go-sqlcmd
go-sqlcmd copied to clipboard
microsoft
Coding guidelines (beyond the Go Style Guide)
This repo follows the Go Style Guide [1], this issue we will use track specific styles beyond that covered in the general Go Style Guide, which we'll then write up as a CODING_GUIDELINES.md
[1] https://google.github.io/styleguide/go/
I've used a coding style for dependency injection using "handler func(err error)", which is pretty abstract.
func Initialize(handler func(err error)) { errorCallback = handler }
PR feedback:
"would it be a bit cleaner to define an interface or type for this handler func instead of using raw func(err error) everywhere?" - David
"I think I agree with you (this is what I'd do in c#), but would using interface encourage tighter coupling. Is the idomatic golang apporach for the caller to define the interface." - Stuart
Another topic around coding guidelines around Platform Abstraction, should we use runtime.GOOS == "windows" etc. or the filename_windows, filename_darwin, filename_linux approach:
Comment wrt pal.go:
"is there really value in splitting these up instead of having a single function that takes the variable name? This function could accept a map instead of an array, and pass the keys as the var names and the values as the var values. The pal function that exports the environment variable would parse the values and do any escaping of special characters." - David
"The value is code coverage reporting . I'd like to promote a culture that a lot of the files in this repo are at 100% "go test" code coverage (already "most" files are), (I want to add a "100PercentClub" build pipeline check so a list (or source file metadata tag?) of all files that are at 100% code coverage to never drop below 100%).
There is 2nd reason, the other approach is to use runtime.GOOS == "windows" etc., but this generates a style failure in go vet (and IDEs), because it is flagged as "unreachable" so you never get the little "green tick" for the source file being 100% good. " - Stuart
Should tests standarize on a specific assert library e.g. github.com/stretchr/testify/assert
Also in the Go Style Guide, what does "invalid local style" / "Use of assertion-based testing libraries" mean:
https://google.github.io/styleguide/go/guide#local-consistency
" Examples of invalid local style considerations:
- Line length restrictions for code
- Use of assertion-based testing libraries "