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

Fix various bugs #25

Merged
merged 4 commits into from
Feb 27, 2017
Merged

Fix various bugs #25

merged 4 commits into from
Feb 27, 2017

Conversation

zhangchn
Copy link
Contributor

This PR undoes the commit e93b212; Fix issues #23 and #24.

Copy link
Member

@groob groob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

undo the context import; everything else looks to be correct.

@@ -10,7 +10,7 @@ import (
"github.com/go-kit/kit/endpoint"
kitlog "github.com/go-kit/kit/log"
kithttp "github.com/go-kit/kit/transport/http"
"golang.org/x/net/context"
"context"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't make this change. Even though go-kit has switched to stdlib context, we can't yet.
There are lots of places where we import "golang.org/x/net/context" in this project as well as other repositories in micromdm. It will take a bit of a coordinated effort to shift to stdlib context, and I'm not prepared to do it until go kit issues a 1.0.0 release in a few weeks.

Instead, use glide install to download the correct dependencies to the /vendor folder and the project should compile. The correct version of go-kit is already pinned in the lock file.

See https://github.com/Masterminds/glide

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better if something like semver were incorporated to manage all the versioning constraints?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what glide is for :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how familiar you are with Go, but the dependency management is not great. There's lots of tools glide, dep, godep, govendor etc which are all trying to manage versions.

@groob groob merged commit 43c5ee6 into micromdm:master Feb 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants