search

kothar / brotli-go

Not maintained -> see itch.io's more up-to-date fork 👉
https://github.com/itchio/go-brotli
MIT License
82 stars 9 forks source link

Support custom dictionaries for encoding/decoding. #41

Closed millisecond closed 7 years ago

millisecond commented 7 years ago

I was able to move encoding changes to only modify encode_go but not seeing a parallel with decode. See that a decode_go.c structure might work, but beyond my C skills :)

kothar commented 7 years ago

Hi, sorry for not reviewing this before, thanks @fasterthanlime for picking it up.

Regarding #40 - The CI script does some work to move the code into the right path so that the gopkg.in package import works (also thanks to Amos), so don't worry about that - all the tests pass, so it's just that linter check that needs fixing. You can run the linter yourself to check locally:

go get -u github.com/golang/lint/golint
golint ./...

Thanks again for contributing!

millisecond commented 7 years ago

Fixed the lint issue and it's still failing on some not-found thing. I'm also seeing that locally but on a different laptop than I authored it on. Not seeing why it would fail, but will check this weekend when I'm back on my main dev env.

fasterthanlime commented 7 years ago

Offending lines on this job seem to be:

./brotli_test.go:155: undefined: enc.CompressBufferDict
./brotli_test.go:166: undefined: dec.DecompressBufferDict
millisecond commented 7 years ago

Yeah, but in the branch I'm pulling from those functions exist, here's the enc one: https://github.com/millisecond/brotli-go/blob/master/enc/encode.go#L182

fasterthanlime commented 7 years ago

Oh yeah I wasn't blaming you :) I think there still might be something wrong with the CI setup. Go is notoriously hard to set up right.. But I think Travis CI got some golang support recently? Not sure if that's usable with the way brotli-go is set up.

kothar commented 7 years ago

@fasterthanlime was correct, the CI setup was broken, possibly due to my introduction of GVM to build with newer versions of Go than Travis used to support. Some weirdness was happening with two different go paths, and the build directory not being in the active GOPATH. 🤕

I've removed my overrides, and the build script now uses the default behaviour for installing go. I'm closing and re-opening this PR to get Travis to start a new build. 🤞

kothar commented 7 years ago

Thanks again both of you, sorry for the broken CI build!

millisecond commented 7 years ago

Hey guys, back in the saddle now. Thanks for taking that to the finish line!