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

samples: move go.mod into samples root, making samples its own module #2057

Merged
merged 3 commits into from
May 13, 2019

Conversation

eliben
Copy link
Member

@eliben eliben commented May 13, 2019

Updates #2048 - moves the go.mod file from samples/appengine to samples.

Note the replace gocloud.dev => ../ added to the go.mod. Without this, running go test ./... in samples/ results in many errors like:

can't load package: package gocloud.dev/samples/server: unknown import path "gocloud.dev/samples/server": ambiguous import: found gocloud.dev/samples/server in multiple modules:
	gocloud.dev/samples (/home/eliben/eli/go-cloud/samples/server)
	gocloud.dev v0.13.0 (/home/eliben/eli/go/pkg/mod/[email protected]/samples/server)

The symptom and solution is explained by bcmills in ugorji/go#279 (which refers also to golang/go#27899). The new go.mod points to gocloud.dev v0.13, which also provides these packages - so the go command is confused - it sees the same package(s) provided by two different modules.

The ugorji/go solution was to use a pseudo-version pointing at an existing commit in the core module which removes the packages - this removes the ambiguity. In our case, there is no existing commit yet - so I'm using a replace line. The replace line should be unnecessary when we release a new CDK version.

This has interesting implications for #886 - we'll likely have to do the same when we split out providers to their own modules and retain replace lines until a new release.

@googlebot googlebot added the cla: yes Google CLA has been signed! label May 13, 2019
@eliben eliben requested review from zombiezen and vangent May 13, 2019 13:06
@codecov
Copy link

codecov bot commented May 13, 2019

Codecov Report

Merging #2057 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2057   +/-   ##
======================================
  Coverage    76.8%   76.8%           
======================================
  Files          82      82           
  Lines        9868    9868           
======================================
  Hits         7579    7579           
  Misses       1729    1729           
  Partials      560     560
Impacted Files Coverage Δ
pubsub/kafkapubsub/kafka.go 83.45% <0%> (-0.74%) ⬇️
internal/retry/retry.go 100% <0%> (+11.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e22a7f...9238cc9. Read the comment docs.

@vangent vangent changed the title samples: move go.mod into samples root, making sampels its own module samples: move go.mod into samples root, making samples its own module May 13, 2019
@eliben eliben merged commit 714b3ed into google:master May 13, 2019
eliben added a commit that referenced this pull request May 14, 2019
@zombiezen
Copy link
Contributor

Can you also update the VSCode workspace? Otherwise IntelliSense breaks.

@eliben
Copy link
Member Author

eliben commented May 14, 2019

@zombiezen, sure thing #2076

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA has been signed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants