joomcode / errorx

A comprehensive error handling library for Go
MIT License
1.12k stars 29 forks source link

Use go 1.13 features to improve IsOfType checks #34

Closed PeterIvanov closed 3 years ago

PeterIvanov commented 3 years ago

The only hiccup here is that code for earlier versions must remain unchanged, it doesn't look all that pretty in source code. Also, I could use any ideas for more tests.

g7r commented 3 years ago

@PeterIvanov, I suggest that public API functions shouldn't be located under //+build .... The reason for that is to not separate documentation for different Go versions. We should document that the behaviour of these functions depends on Go version.

PeterIvanov commented 3 years ago

@g7r as implementation stands now, I could move only burrowForTyped (renamed) under build tag, but it may not be better for core readability. But since implementation changed, it may be better just to keep them separate, and in some later release simply drop support of go version prior to 1.13. At this point, code structure would become plain again.

g7r commented 3 years ago

@PeterIvanov, I suggest something like that:

--- interface.go
// no build tag

// godoc stating that the behaviour is different when using 1.12 and below and 1.13 and above Go versions.
func IsOfType(...) bool {
    return isOfType(...)
}

--- impl_1.12.go
//+build !go1.13

func isOfType(...) bool {
  // actual implementation for 1.12 and below
}

--- impl_1.13.go
//+build go1.13

func isOfType(...) bool {
  // actual implementation for 1.13 and above
}

We may inline implementation after dropping 1.12 Go version support. BTW, why delay dropping 1.12 support? Maybe we should use the precedent to drop it? 1.12 is two years old today https://blog.golang.org/go1.12 :)

PeterIvanov commented 3 years ago

It would be quite ironic do drop 1.12 at the exact same moment as errorx finally catches up with go 1.13 :) There's no dire need for that at this point, so I would at least delay a little bit.