42wim / matterbridge

bridge between mattermost, IRC, gitter, xmpp, slack, discord, telegram, rocketchat, twitch, ssh-chat, zulip, whatsapp, keybase, matrix, microsoft teams, nextcloud, mumble, vk and more with REST API (mattermost not required!)
Apache License 2.0
6.66k stars 618 forks source link

get rid of vendor directory #2173

Closed siddarthkay closed 2 months ago

siddarthkay commented 2 months ago

Summary

checking in the vendor folder for a golang project is similar to committing node_modules in a js project. This PR gets rid of the vendor directory from this repository and adds /vendor to .gitignore

Why

Having a vendor directory checked in makes reviewing and rebasing upgrading PRs with a golang bump or a dependency upgrade a nightmare due to the amount of files changed. Since most IDEs are able to generate the vendor directory automatically I think its better to not check in the vendor directory.

I have no clue about the maintainers take on this opinion but I wanted to start the conversation with this PR. Thank you for your time and patience 🙏🏻

codeclimate[bot] commented 2 months ago

Code Climate has analyzed commit 0e9b92db and detected 0 issues on this pull request.

View more on Code Climate.

siddarthkay commented 2 months ago

@42wim : what do you think about this proposal ?

IntGrah commented 2 months ago

Isn't the vendor directory is usually committed to ensure reproducible builds? It's nowhere near as heavy as node_modules

siddarthkay commented 2 months ago

@IntGrah : I agree its not heavy but the number of files are insanely large. Rebasing and upgrading go versions is a nightmare. Look at the diff of this PR.

Screenshot 2024-08-27 at 4 16 06 PM

for reproducibility purposes there are package managers like nix that do the job. Committing the vendor folder doesn't help much for that use case I believe.

42wim commented 2 months ago

Thanks, but the vendor folder is there for reproduceability and when upstream libraries would suddenly be gone, it's not going to get removed.

jakubgs commented 2 months ago

You could just use forks of these dependencies if you worry about original repos disappearing. I do think committing vendor folder is a bad practice, but if it solves a problem for that's fine.