Skip to content

Conversation

@starnop
Copy link
Contributor

@starnop starnop commented Nov 8, 2018

Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

We use the master branch of containernetworking/plugins when install cni , with this PR containernetworking/plugins#219 submitted, we got error info when running the test about cri

install cni...
hack/install/install_cni.sh: line 26: /home/travis/gopath/src/github.com/containernetworking/plugins/build.sh: No such file or directory
make: *** [cri-v1alpha1-test] Error 1

It's so not make sense. We should fix the version that we use instead of relying heavily on the latest Pull Request and upgrade the version manually when we need it.

Ⅱ. Does this pull request fix one issue?

None.

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

None.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@pouchrobot pouchrobot added areas/network kind/bug This is bug report for project size/S labels Nov 8, 2018
@chuanchang
Copy link
Contributor

chuanchang commented Nov 8, 2018

@starnop IMO, it had better to keep consistent with upstream, you know the cni plugin will constantly change, so I think it's not a good idea to give fixed version, what do you think?

BTW, I have an another #2452 to fix it.


# downloads github.com/containernetworking/plugins
go get -u -d "${pkg}"/...
if [ ! -d "${workdir}" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, the size of the source code package is smaller than the size of the .tgz. In addition, we will be freedom to operate the branch . So no need to change it, IMO. WDYT? Anyway, thanks for your advice. :)

@fuweid
Copy link
Contributor

fuweid commented Nov 8, 2018

@starnop IMO, it had better to keep consistent with upstream, you know the cni plugin will constantly change, so I think it's not a good idea to give fixed version, what do you think?

I think we should use the stable version here because the change maybe unstable in master.

Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

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

Labels

areas/network kind/bug This is bug report for project size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants