-
Notifications
You must be signed in to change notification settings - Fork 742
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
Bump containernetworking dependencies only #916
Conversation
3d0cbfc
to
a499272
Compare
}, | ||
) | ||
delIPCnt = prometheus.NewCounterVec( | ||
prometheus.CounterOpts{ | ||
Name: "awscni_del_ip_req_count", | ||
Help: "The number of delete IP address request", | ||
Help: "The number of delete IP address requests", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change primarily for readability ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sentence was not correct.
@@ -13,7 +13,7 @@ | |||
|
|||
package cninswrapper | |||
|
|||
import "github.com/containernetworking/cni/pkg/ns" | |||
import "github.com/containernetworking/plugins/pkg/ns" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have all these packages moved under plugins directory now ? Looking at this PR in isolation is not giving me that insight. Can we still continue to use cni directory packages ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The upstream package was moved in v0.6.0 back in August 2017, so we have to import the files from where they are now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarification. Lets run integration changes post this at least once again before the release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets wait for others to bless this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work Mogrenator!
@@ -15,7 +16,7 @@ require ( | |||
github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf // indirect | |||
github.com/googleapis/gnostic v0.2.0 // indirect | |||
github.com/gregjones/httpcache v0.0.0-20190212212710-3befbb6ad0cc // indirect | |||
github.com/hashicorp/golang-lru v0.0.0-20180201235237-0fb14efe8c47 // indirect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o/ always good to get rid of older non-semver deps.
Description of changes:
ns
package has moved to theplugins
package.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.