Closed jdef closed 7 years ago
@jdef what do you think of taking this even further and just adopt a dependency manager like glide
?
would prefer govendor
On Wed, Apr 19, 2017 at 7:36 PM, Paulo Pires notifications@github.com wrote:
@jdef https://github.com/jdef what do you think of taking this even further and just adopt a dependency manager like glide?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mesosphere/etcd-mesos/pull/109#issuecomment-295493931, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLB_oFvmGX8P3rA0f7K_9qxnfCnWVks5rxpp8gaJpZM4MMDkx .
OK I have govendor
changes and some improvements to the Makefile
ready. Should I push over here or should I create a separate branch/PR?
OK sorry for the forced push, but I wanted to rebase some changes from master
. We now have a successful deploy with code built w/ govendor
, so I'm happy to confirm the dependencies are in a good shape.
Next, I'll execute most tests described in #112.
LGTM @jdef assigning to you now.
Mostly agree. Slightly concerned because I can't tell if the original author was feeling rushed (and perhaps just wanted the code to compile), or if he was working around an error. That said, let's merge what we have now
On Fri, Apr 21, 2017 at 7:20 PM, Paulo Pires notifications@github.com wrote:
@pires commented on this pull request.
In vendor/vendor.json https://github.com/mesosphere/etcd-mesos/pull/109#discussion_r112792479:
@@ -0,0 +1,268 @@ +{
- "comment": "",
- "ignore": "test",
- "package": [
- {
- "checksumSHA1": "E2a5TaCbwqs7PwP1fvfWLhvgr38=",
- "comment": "v2.2.0-rc.0-72-g106d918",
- "path": "github.com/coreos/etcd/Godeps/_workspace/src/github.com/coreos/go-systemd/journal",
Cloning both repositories:
$ git clone git@github.com:mesosphere/etcd-mesos.git $ git clone https://github.com/coreos/etcd.git $ cd etcd && git checkout 106d918dd585a63ee0045740fe040daa44669a77
Diff'ing resources that exist in both directories simultaneously:
$ diff etcd-mesos/Godeps/_workspace/src/github.com/coreos/etcd/etcdserver/stats/ etcd/etcdserver/stats/ diff etcd-mesos/Godeps/_workspace/src/github.com/coreos/etcd/etcdserver/stats/stats.go etcd/etcdserver/stats/stats.go 17c17 < import "github.com/coreos/pkg/capnslog"
import "github.com/coreos/etcd/Godeps/_workspace/src/github.com/coreos/pkg/capnslog"
$ diff etcd-mesos/Godeps/_workspace/src/github.com/coreos/etcd/etcdserver/etcdhttp/httptypes/ etcd/etcdserver/etcdhttp/httptypes/ diff etcd-mesos/Godeps/_workspace/src/github.com/coreos/etcd/etcdserver/etcdhttp/httptypes/errors.go etcd/etcdserver/etcdhttp/httptypes/errors.go 21c21 < "github.com/coreos/pkg/capnslog"
"github.com/coreos/etcd/Godeps/_workspace/src/github.com/coreos/pkg/capnslog"
Now, in Godeps/Godeps.json we have:
{ "ImportPath": "github.com/coreos/go-systemd/journal", "Comment": "v3-11-g6f3d6b0", "Rev": "6f3d6b0accd17b33624ee4df1532a03de6e4d536" },
But in the official code we actually have https://github.com/coreos/ etcd/blob/106d918dd585a63ee0045740fe040daa44669a77/Godeps/_workspace/ src/github.com/coreos/pkg/capnslog/journald_formatter.go#L24.
I conclude there is no code that has been changed other than imports and we should clean this up when adopting etcd v3.x.
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/mesosphere/etcd-mesos/pull/109#discussion_r112792479, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLJt8EURwtFY9tDWLYls1E0OMr1TNks5ryTnKgaJpZM4MMDkx .
NOTE: internal etcd dependencies related to capnslog are being rolled back to etcd-stock versions with this PR and may impact logging (read: may reintroduce regressions). The intent is that these deps will be updated in the near term as part of a migration to etcd 3.x