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

Make conflistDel() behave like conflistAdd() #182

Merged
merged 1 commit into from
Nov 1, 2018

Conversation

mccv1r0
Copy link
Contributor

@mccv1r0 mccv1r0 commented Nov 1, 2018

conflistAdd() finds binaries differently than conflistDel().
Make the two call find binaries the same way.

Fixes #179

Signed-off-by: Michael Cambria mcambria@redhat.com

@coveralls
Copy link

coveralls commented Nov 1, 2018

Pull Request Test Coverage Report for Build 468

  • 0 of 5 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 46.413%

Changes Missing Coverage Covered Lines Changed/Added Lines %
multus/multus.go 0 5 0.0%
Totals Coverage Status
Change from base Build 467: -0.05%
Covered Lines: 414
Relevant Lines: 892

💛 - Coveralls

Copy link
Member

@s1061123 s1061123 left a comment

Choose a reason for hiding this comment

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

Looks good to me. I add a minor comment.

multus/multus.go Outdated
@@ -140,11 +140,13 @@ func conflistAdd(rt *libcni.RuntimeConf, rawnetconflist []byte, binDir string, e
return result, nil
}

func conflistDel(rt *libcni.RuntimeConf, rawnetconflist []byte, binDir string) error {
//func conflistDel(rt *libcni.RuntimeConf, rawnetconflist []byte, binDir string) error {
Copy link
Member

Choose a reason for hiding this comment

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

How about to remove it?

conflistAdd() finds binaries differently than conflistDel().
Make the two call find binaries the same way.

Fixes k8snetworkplumbingwg#179

Signed-off-by: Michael Cambria <mcambria@redhat.com>
Copy link
Member

@s1061123 s1061123 left a comment

Choose a reason for hiding this comment

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

Looks good to me, but it seems to be conflicts. Could you rebase to head?

@s1061123 s1061123 merged commit aa7e000 into k8snetworkplumbingwg:master Nov 1, 2018
@s1061123
Copy link
Member

s1061123 commented Nov 1, 2018

merged, thanks again!

@mccv1r0 mccv1r0 deleted the binDirs branch November 2, 2018 20:29
@dougbtv dougbtv added the release-v3 PRs to make it in release branch label Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-v3 PRs to make it in release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants