Skip to content
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

support go mod #905

Closed
wzjgo1 opened this issue Aug 16, 2019 · 10 comments
Closed

support go mod #905

wzjgo1 opened this issue Aug 16, 2019 · 10 comments
Labels

Comments

@wzjgo1
Copy link

wzjgo1 commented Aug 16, 2019

No description provided.

@peterbourgon
Copy link
Member

peterbourgon commented Aug 16, 2019

Go kit is not incompatible with projects that use modules, so in that sense this is already done.

But (as I've said in the past) the problem with adding a go.mod to this project itself is that I explicitly don't want to enforce minimum version selection on my consumers. I want Go kit to always be compatible with the latest (stable) releases of all of its dependencies, and I want to fix problems that would prevent that from being true as soon as they come up.

So, how do we square this circle? Is there a way to have a go.mod that is continuously and automatically updated with the latest stable versions of all dependencies, and can alert me on build or test failures?

@stefanvanburen
Copy link

you could possibly configure https://dependabot.com for go modules, and have it auto-merge on passing builds

@peterbourgon
Copy link
Member

I guess I left out the other side of the equation, which is that within a major version, upgrades are presumed to be backwards-compatible, so an end user is free to use v1.2.0 of a dep that I have pinned at v1.1.0, if necessary; and that breaking changes (should) necessarily coincide with major version upgrades, which are entirely different imports that can coexist in a compilation unit.

So: maybe it's fine. I would still not like to take on the maintenance burden of tracking major version bumps over time in my go.mod, but perhaps I'm overestimating how often that will happen.

@heat
Copy link

heat commented Aug 26, 2019

O got this erro
github.com\go-kit\kit@v0.9.0\tracing\opentracing\grpc.go:10:2: unknown import path "google.golang.org/grpc/metadata": cannot find module providing package google.golang.org/grpc/metadata

Problem

go get google.golang.org/grpc/metadata: unrecognized import path "google.golang.org/grpc/metadata" (https fetch: Get https://google.golang.org/grpc/metadata?go-get=1: net/http: TLS handshake timeout)

@LasTshaMAN
Copy link

LasTshaMAN commented Sep 21, 2019

I want Go kit to always be compatible with the latest (stable) releases of all of its dependencies, and I want to fix problems that would prevent that from being true as soon as they come up.

Would it be too routine to update go mod dependencies from time to time manually ?

Moving to go modules would make kit more developer-friendly, so that they will be more likely to check it out and contribute ...
Not everybody "is ready" to run go get (or some bash scripts) in his/her GOPATH just to make a small contribution to go-kit.

Also, go modules allows devs to work outside of GOPATH, and now working outside of GOPATH is the default way to work with Go. Thus the following sequence of actions becomes common:

  • having cloned go-kit in my home directory and fruitlessly having tried to build it, now I have to remove it and clone go-kit once again in GOPATH and re-import my project in IDE as well.

@LasTshaMAN
Copy link

Another consideration is - suppose you want to contribute some feature, you update all the dependencies, then you build go-kit project and the build (or some tests) fails because 3rd party dependency broke backward-compatibility or something in its latest commit.

Now you have to stop working on your feature and investigate what is going with the failing tests, cause you can't even make your CI pipeline pass. And, if you want to contribute this single feature to go-kit and move on, you are probably not gonna bother with fixing these integration issues.

If you were to have builds based on go modules, on the other hand, then you almost never will encounter this scenario.
Thus, go modules will allow us to work on integration issues separately from all the rest.

@LasTshaMAN
Copy link

Also, trying to run update_deps.bash on go v1.13 I'm getting an error:

mbp-iurii:kit iurii$ go version
go version go1.13 darwin/amd64
mbp-iurii:kit iurii$ ./update_deps.bash
can't load package: named files must be .go files: bufio
can't load package: named files must be .go files: github.com/go-kit/kit/log

It works with go v1.11, so I'm guessing the Go team changed something in go list command, and we have to fix around it as well,
and maybe that's not the last time this happens.

@LasTshaMAN
Copy link

And, running tests locally (with the latest version of dependencies) I see some of them fail seemingly for integration reasons:

--- FAIL: TestProcess/foo (0.03s)
        --- FAIL: TestProcess/foo/flat (0.01s)
            main_test.go:63: - import "context"
            main_test.go:63: - import "encoding/json"
            main_test.go:63: - import "errors"
            main_test.go:63: - import "net/http"
            main_test.go:63: - import "github.com/go-kit/kit/endpoint"
            main_test.go:63: - import httptransport "github.com/go-kit/kit/transport/http"
            main_test.go:63: + import (
            main_test.go:63: + 	"context"
            main_test.go:63: + 	"encoding/json"
            main_test.go:63: + 	"errors"
            main_test.go:63: + 	"net/http"
            main_test.go:63: +
            main_test.go:63: + 	"github.com/go-kit/kit/endpoint"
            main_test.go:63: +
            main_test.go:63: + 	httptransport "github.com/go-kit/kit/transport/http"
            main_test.go:63: + )

Looks like some code-generating library changed output format ...
In this PR you can find failing CI pipeline, and, funny enough, it fails for another reason - also having to do with some latest changes in the libraries go-kit depends on (looks like github.com/openzipkin-contrib/zipkin-go-opentracing has changed its API) ...

So, it is a frustrating (if not blocking) experience for a contributor ...

@sagikazarmark
Copy link
Contributor

Fixed in #945

@LasTshaMAN
Copy link

I believe every concern I've raised in this issue is fixed, thanks to @ChrisHines!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants