-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test not passing #1
Comments
There's a bug in Go 1.12 that prevents it from sorting these multi-type slices properly. See golang/go#30398 If you use the |
I'll wait for the next release. |
You can get it working by changing a single line in the stdlib. Open @@ -167,7 +167,7 @@ func compare(aVal, bVal reflect.Value) int {
if c, ok := nilCompare(aVal, bVal); ok {
return c
}
- c := compare(reflect.ValueOf(aType), reflect.ValueOf(bType))
+ c := compare(reflect.ValueOf(aVal.Elem().Type()), reflect.ValueOf(bVal.Elem().Type()))
if c != 0 {
return c
} The test should then pass. |
Indeed it fixed! And I learned something amazing in the process: Go standard library code is recompiled every time if need be. It's not just a large binary .so or .dll file that you need to manually recompile after every change. So just altering that one line you told me and issuing another $ go test
PASS
ok github.com/ww9/intersort 0.335s This is amazing, it feels like I'm coding an interpreted language, not a compiled one! Thank you for your attention! |
I knew that Go compilation was fast and I know there's a lot of caching in the process that makes this faster, including filesystem level cache provided by SO. But still, this gives me pascal/TCC speed tier vibes. |
My zealotry notwithstanding, I'll take the liberty to re-open this issue so it can help others trying to play with this meme sort. At least until next release of Go supports it. I'll take no offense (and why would I) if you chose to close it. Thanks |
Indeed, Go is pretty amazing! :) I agree, let's leave this open until the fix makes it into a release. |
Go 1.12.1 was released today, including my fix. So tests should now pass if you are running the latest Go release. :) I will mention this in the README. |
Thanks! |
Hi, not sure if I'm doing something wrong but the test isn't passing for me :(
The text was updated successfully, but these errors were encountered: