apple / pkl-go

Pkl bindings for the Go programming language
https://pkl-lang.org/go/current/index.html
Apache License 2.0
261 stars 24 forks source link

Remove test dependencies from github.com/apple/pkl-go/pkl #2

Closed 3052 closed 7 months ago

3052 commented 8 months ago

EDITED by @bioball:

When this package is imported, it brings along test dependencies like github.com/stretchr/testify, which shouldn't be in there.

Maybe we need to move the tests into their own package (e.g. package pkl_test)

Original post:

current go.sum is over 700 lines:

https://github.com/apple/pkl-go/blob/main/go.sum

compare with encoding/json, which obviously uses no third party packages:

https://godocs.io/encoding/json?view=imports

or even tidwall/gjson, with a go.sum of 4 lines:

https://github.com/tidwall/gjson/blob/master/go.sum

or BurntSushi/toml, no third party packages:

https://github.com/BurntSushi/toml/blob/master/go.mod

or ghodss/yaml, with a go.sum of 3 lines:

https://github.com/ghodss/yaml/blob/master/go.sum

dmitshur commented 8 months ago

Updating to newer released versions of the current direct dependencies and running go mod tidy would remove 751 lines from the go.sum file. (That said, it's worth taking into account that the number of lines in a go.sum file is a fairly indirect measure of module graph size.)

bioball commented 8 months ago

Most of these dependencies are only used for pkl-gen-go, including the spf13 modules. In a normal Go app, you would only import pkl/, and most of these dependencies don't get brought along.

I'll keep this issue up though because we still include thing like github.com/stretchr/testify. We probably need to make the tests part of a different package.

3052 commented 8 months ago

up to you. but as someone who codes Go daily, both professional and personal, I usually wont touch any module with a go.sum over 100 lines, so something to think about

bioball commented 8 months ago

Why is that? Go will eliminate unused dependencies, so dependencies like spf13 won't actually show up in your project, nor your go.sum (unless you use pkl-gen-go as a library). Are there downsides that I'm not aware of?

3052 commented 8 months ago

its just different styles of programming. I am a minimalist, and I carefully vet any third party dependencies. so when I see a module with literally hundreds of lines in go.sum, and then compare similar packages (listed above), that have at most 4 lines, it gives me pause. it says to me the authors either didn't think about this factor, or did think about it and intentionally decided that it wasn't important.

also another user (in fact a member of the Go team) mentioned that a simple go mod tidy could possibly remove 96% of the lines. so the fact that you've ignored or missed that comment, and decided to instead try to argue and justify the bloat, is concerning

bioball commented 8 months ago

FWIW: JSON, YAML, and TOML are all static languages. They're somewhat similar in that they're also used for configuration, but they're quite different because they only need to be parsed. Pkl is programming language that gets parsed, then evaluated to produce an output.

With that being said, the only library our users will need to pull in is github.com/vmihailenco/msgpack, and its transitive dependencies. The extra libraries that you see in our go.sum are because of ancillary packages that are also hosted in this repo (for example. cmd/pkl-gen-go, which is a binary that we ship that does codegen).

But, you're right that there's some bloat here, because I just tested this and a downstream project is pulling in some testing libraries (e.g. github.com/stretchr/testify). We'll clean that up.

3052 commented 7 months ago

thats not great, but a hell of a lot better than it was!

thank you!