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

Update dependency on ugorji/go/codec #3655

Merged
merged 2 commits into from
Oct 9, 2015

Conversation

wojtek-t
Copy link
Contributor

@wojtek-t wojtek-t commented Oct 8, 2015

Rationale behind this change:
I are going to use ugorji/go/codec in Kubernetes too. However - in the version that you are currently using there were some issues (that k8s code discovered) and fixing them made some backward incompatible changes. And having k8s and etcd depend on different version of that dependency that are incompatible would be a nightmare.

@xiang90 @yichengq

@wojtek-t
Copy link
Contributor Author

wojtek-t commented Oct 8, 2015

@xiang90 @yichengq - not sure what I am doing wrong that tests are not passing. Can you please hint me what I should fix?

@@ -14,7 +14,7 @@

package client

//go:generate codecgen -r "Node|Response|Nodes" -o keys.generated.go keys.go
//go:generate codecgen -d 1819 -r "Node|Response|Nodes" -o keys.generated.go keys.go
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this? is the number supposed to be random when each time the code is generated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't set it explicitly - the it is random based on the current time, which makes the generated code differ every time it's generated. I think having it always be the same is expected.

@yichengq
Copy link
Contributor

yichengq commented Oct 8, 2015

we use Godeps/ as import path in etcd. This difference(https://github.com/coreos/etcd/pull/3655/files#diff-6dcf0cfb1426fb0ea1d349820d7622a3L11) makes it failed. You could use godep save -r to let godep do it for you.

BTW, the format of commit title needs to be changed according to https://github.com/coreos/etcd/blob/master/CONTRIBUTING.md#format-of-the-commit-message

@wojtek-t
Copy link
Contributor Author

wojtek-t commented Oct 9, 2015

@yichengq - I'm not sure if I understood. What I've done to update it was:

go get -u github.com/ugorji/go/codec
godep update github.com/ugorji/go/codec

We also use Godeps in k8s and this is what we do there. Because of some reason that doesn't work here.

Regardinc commit names - PTAL

Update ugorji/go/codec dependency to the newer version (a bunch of fixed were made).
Regenerate code for unmarshaling key response with a new version of
ugorji/go/codec
@wojtek-t
Copy link
Contributor Author

wojtek-t commented Oct 9, 2015

OK - it seems that the problem was with generated imports - I've changed on of those and it seems to work now.

@yichengq - PTAL

@yichengq
Copy link
Contributor

yichengq commented Oct 9, 2015

LGTM. Thanks for updating this!

yichengq added a commit that referenced this pull request Oct 9, 2015
Update dependency on ugorji/go/codec
@yichengq yichengq merged commit ce45159 into etcd-io:master Oct 9, 2015
@wojtek-t
Copy link
Contributor Author

wojtek-t commented Oct 9, 2015

@yichengq - thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants