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

Bring go-netbox into this repo #107

Closed
holmesb opened this issue Jan 8, 2022 · 5 comments
Closed

Bring go-netbox into this repo #107

holmesb opened this issue Jan 8, 2022 · 5 comments

Comments

@holmesb
Copy link
Contributor

holmesb commented Jan 8, 2022

Hi,

A suggestion: to avoid having to switch go-netbox client en-masse during PRs when changes to it are needed, might be worth considering moving the client into this repo. Eg in PR #106, I have switched to using my forked go-client so the PR can be tested. Would be obviated and the client & terraform provider kept in-sync if go-netbox was moved here. Easier PR process. fbreckle/go-netbox serves this provider exclusively and is not general purpose.

Cheers

@fbreckle
Copy link
Collaborator

Hm. I see the issue, but I think it would be VERY bad practice to do so. They are separate things and should be separated.

Pinging @FlxPeters to get a second opinion on this.

@FlxPeters
Copy link
Member

To be honest I'm not sure if its a good or bad idea. Both have their pros and cons.
Managing the lib outside of the provider is the clean and best practice way. In a perfect world we wouldn't need a custom provider but the official one is not good enough.

The question is, is there any other user of the lib code than this repo? If not, it would be a possible solution.
If we cannot guarantee it i would stay with the external repo.

@holmesb
Copy link
Contributor Author

holmesb commented Jan 10, 2022

If there's a chance fbreckle/go-netbox will become the main go client, then I can understand keeping it separate. It has many improvements over the "official" one.

@fbreckle
Copy link
Collaborator

I don't think the client will become the main client, because it has some changes specific to this provider.

I thought some more about it and remembered that Go has some replace directive for the module file. I think this problem can be solved with that.

See https://go.dev/ref/mod#go-mod-file-replace or some google-fu like https://stackoverflow.com/questions/56671133/how-to-use-a-forked-module-with-versioned-go-modules-v1-11-go111module-on

For your PRs, you would then just add your fork to the go.mod once instead of changing every file.

@holmesb
Copy link
Contributor Author

holmesb commented Jan 10, 2022

Good enough. Thx @fbreckle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants