github.com/shurcooL/issuesapp/...

Why own `UserSpec`, instead of `sourcegraph.UserSpec` #30

Closedkeegancsmith opened this issue 3 years ago
keegancsmith commented 3 years ago

For example the notifications API uses a issues.UserSpec, which is a bit surprising. cc @shurcool

Write Preview Markdown
dmitshur commented 3 years ago ยท edited

It allowed me to have greater flexibility and control in the issues service definition. The UserSpec was added fairly late in the dev cycle.

The primarily reason I defined it here instead of pulling in sourcegraph.UserSpec is because I wanted the service definition package to be as lightweight as possible, as it currently is:

https://godoc.org/github.com/shurcooL/issues?import-graph&hide=1

It imports only one package that isn't standard Go library golang.org/x/net/context, which is necessary and minimal.

If I were to try to use sourcegraph.UserSpec directly in this package, it would cause many, many unnecessary packages to be imported indirectly because of grpc, as you can see here:

https://godoc.org/github.com/sourcegraph/go-sourcegraph/sourcegraph?import-graph&hide=1

That would make it much more difficult for implementations of issues.Service to exist. For example, here's an implementation I made that implements issues.Service from a WordPress export XML file.

In summary, the goal of this package is to define a general and useful interface. The goal of other specific packages that implement the interface is to do so according to their needs; that's why the kv package imports go-sourcegraph and uses sourcegraph.UserSpec there. Does that make sense?

Write Preview Markdown
keegancsmith commented 3 years ago

Yes that makes sense. It is frustrating the go-sourcegraph is so heavy weight, especially since most uses don't care at all about gRPC, but rather just the structs themselves. Thanks!

Write Preview Markdown
keegancsmith closed this 3 years ago
to comment.