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

Remove circular dependency on codec submodule #319

Closed
wants to merge 1 commit into from

Conversation

dlaguerta
Copy link

@dlaguerta dlaguerta commented Sep 12, 2019

Requiring the root module in the codec submodule creates a circular dependency, which causes go mod issues.
By removing it as required in codec/go.mod, I believe people should not see issues.

@dlaguerta dlaguerta mentioned this pull request Sep 12, 2019
@dlaguerta
Copy link
Author

@ugorji -- could you take a look?

@ugorji
Copy link
Owner

ugorji commented Sep 14, 2019

I can't accept this PR without understanding what it did, exactly what it solves, or the implications.

@1995parham
Copy link

@ugorji I think this PR resolve #318 by removing the circular dependency between Go modules that are defined in the project.

@dlaguerta
Copy link
Author

Like @1995parham mentioned, clients using this module are getting errors due to the circular dependency within this project (#318).
The circular dependency defined in the go.mod mod file of go/codec also causes inconsistent changes to a client's go.mod file when nothing has actually changed. For example, when running go test, clients will see this change:

-	github.com/ugorji/go/codec v1.1.7 // indirect
+	github.com/ugorji/go v1.1.7 // indirect

running go mod tidy will revert this change, but clients would have to always run go mod tidy after, for example, running go test.
The better fix would be to just remove the dependency on the root module (ugoriji/go) from the codec mod. This PR resolves #318.

@ChrisRx
Copy link

ChrisRx commented Nov 9, 2019

This change would be greatly appreciated. The Go modules defined within the project are setup incorrectly and this would go a long way to helping with some of the downstream problems intrinsic to Go modules themselves that people are experiencing.

With that said, after reading through a lot of the issues to get backstory, I think a overall better solution would be to also only use one Go modules at the project root, place a doc.go file (or whatever), and have codec be a sub-package (and bumping the module to v2, which would be very important). All of the headaches experienced with the project with regard to Go modules are due to the incorrect usage and subsequent definition of multiple Go modules also, so I think if the v2 could just eliminate the unnecessary use of multiple modules (and the unnecessary folder structure) it would help to guard against these issues in the future.

In any case, I would really like to see this PR get merged to address the go mod tidy issue described and would like to help in any way that I can. Thank you for your time and patience.

mrhwick pushed a commit to DataDog/viper that referenced this pull request Feb 27, 2020
@ugorji
Copy link
Owner

ugorji commented Sep 11, 2020

Please see thread #299

This gives the context and history, explaining what influenced the decision to make the committed fix.

It seems that things are working well now. Closing. Please ping/reopen if needed.

@ugorji ugorji closed this Sep 11, 2020
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

Successfully merging this pull request may close these issues.

4 participants