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.

Sign up for GitHub

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

Jump to bottom

Get rid of vendoring #544

Closed
jaypipes opened this issue Mar 12, 2024 · 8 comments
Closed

Get rid of vendoring #544

jaypipes opened this issue Mar 12, 2024 · 8 comments

Comments

@jaypipes
Copy link
Contributor

Hi! I'm wondering if you would be OK with a pull request that removes the vendor/ directory and uses only Go modules for dependency management?

We (Microsoft Network Cloud) have a mirror of this repo internally for software supply chain purposes and the vendor/ directory makes the mirror maintenance unnecessarily painful. Modern Go and modules are a cleaner, more maintainable way to manage dependencies...
@adrianchiris
Copy link
Contributor

Hi @jaypipes, im ok with this change.

@SchSeba @Eoghan1232 WDYT?

i dont remember if for openshift this is needed.

@Eoghan1232
Copy link
Collaborator

I am happy for it to go :)
but afaik, it's a requirement on the RH side? @SchSeba

@jaypipes
Copy link
Contributor Author

@ffromani what say you, Red Hatter? :)

Also, Francesco, I'll be submitting similar PRs to remove vendoring from all the topologyawarewg repos!

@ffromani
Copy link

@ffromani what say you, Red Hatter? :)

Also, Francesco, I'll be submitting similar PRs to remove vendoring from all the topologyawarewg repos!

Can't comment for this repo (@SchSeba / @zeeke are better candidates), but I'm willing to experiment for the repos I maintain

@zeeke
Copy link
Member

zeeke commented Mar 13, 2024

Looks good to me!

We already have examples of upstream-no-vendor/downstream-vendored projects.
(https://github.com/metallb/metallb-operator / https://github.com/openshift/metallb-operator).

Getting rid of the vendor folder in opeshift/* forks requires a little more work (don't have all the details ATM) to adjust the delivery pipeline, but it can be unrelated to this repository

@bn222
Copy link
Contributor

bn222 commented Mar 14, 2024

I'm fully on board with this. We should do it in all k8snetworkplumbingwg repos

jaypipes added a commit to jaypipes/knp-multus-cni that referenced this issue Apr 7, 2024
@jaypipes
remove vendoring, use Go modules by default
d358722 
Removes the vendor/ directory and all vestiges of life before Go
modules.

Issue k8snetworkplumbingwg/sriov-network-device-plugin#544

Signed-off-by: Jay Pipes <[email protected]>
@jaypipes jaypipes mentioned this issue Apr 7, 2024
Closed
jaypipes added a commit to jaypipes/knp-sriov-network-device-plugin that referenced this issue Apr 7, 2024
@jaypipes
remove vendoring, use Go modules by default
999ef39 
Removes the vendor/ directory and all vestiges of life before Go modules.

Issue k8snetworkplumbingwg#544

Signed-off-by: Jay Pipes <[email protected]>
@jaypipes jaypipes mentioned this issue Apr 7, 2024
Merged
@jaypipes
Copy link
Contributor Author

@bn222 @zeeke hello! Any chance I can get reviews on the two PRs submitted that remove vendoring from multus-cni and sriov-network-device-plugin? Thanks much in advance! :)

jaypipes added a commit to jaypipes/knp-sriov-network-device-plugin that referenced this issue Apr 16, 2024
@jaypipes
remove vendoring, use Go modules by default
1db4ebc 
Removes the vendor/ directory and all vestiges of life before Go modules.

Also updates pkg/utils/testing.go to properly account for non-RHEL
systems, which typically store the pci.ids file at
/usr/share/misc/pci.ids instead of /usr/share/hwdata/pci.ids.

Issue k8snetworkplumbingwg#544

Signed-off-by: Jay Pipes <[email protected]>
jaypipes added a commit to jaypipes/knp-sriov-network-device-plugin that referenced this issue Apr 16, 2024
@jaypipes
remove vendoring, use Go modules by default
88e71c4 
Removes the vendor/ directory and all vestiges of life before Go modules.

Also updates pkg/utils/testing.go to properly account for non-RHEL
systems, which typically store the pci.ids file at
/usr/share/misc/pci.ids instead of /usr/share/hwdata/pci.ids.

Addresses golint warnings around grpc.DialContext being deprecated by
replacing calls with grpc.NewClient.

Issue k8snetworkplumbingwg#544

Signed-off-by: Jay Pipes <[email protected]>
jaypipes added a commit to jaypipes/knp-sriov-network-device-plugin that referenced this issue May 25, 2024
@jaypipes
remove vendoring, use Go modules by default
2083067 
Removes the vendor/ directory and all vestiges of life before Go modules.

Also updates pkg/utils/testing.go to properly account for non-RHEL
systems, which typically store the pci.ids file at
/usr/share/misc/pci.ids instead of /usr/share/hwdata/pci.ids.

Issue k8snetworkplumbingwg#544

Signed-off-by: Jay Pipes <[email protected]>
jaypipes added a commit to jaypipes/knp-multus-cni that referenced this issue May 25, 2024
@jaypipes
remove vendoring, use Go modules
8740dd3 
Removes the vendor/ directory and all vestiges of life before Go modules.

Issue k8snetworkplumbingwg/sriov-network-device-plugin#544

Signed-off-by: Jay Pipes <[email protected]>
@jaypipes jaypipes mentioned this issue May 25, 2024
Closed
jaypipes added a commit to jaypipes/knp-multus-cni that referenced this issue Jun 7, 2024
@jaypipes
remove vendoring, use Go modules
f6a9112 
Removes the vendor/ directory and all vestiges of life before Go modules.

Issue k8snetworkplumbingwg/sriov-network-device-plugin#544

Signed-off-by: Jay Pipes <[email protected]>
@SchSeba
Copy link
Collaborator

SchSeba commented Aug 19, 2024

closing #548

@SchSeba SchSeba closed this as completed Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@jaypipes @zeeke @SchSeba @adrianchiris @ffromani @Eoghan1232 @bn222