Skip to content
This repository has been archived by the owner on Oct 29, 2021. It is now read-only.

Remove Core from Module type #77

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Conversation

u5surf
Copy link
Contributor

@u5surf u5surf commented Oct 2, 2021

Fixes #15

I supposed to rewrite TestModuleFromCore following #12 (comment)
Signed-off-by: yugo-horie u5.horie@gmail.com

@u5surf u5surf requested review from a team, bogdandrutu and jpkrohling and removed request for a team October 2, 2021 00:21
@@ -64,7 +64,6 @@ type Module struct {
Import string `mapstructure:"import"` // if not specified, this is the path part of the go mods
GoMod string `mapstructure:"gomod"` // a gomod-compatible spec for the module
Path string `mapstructure:"path"` // an optional path to the local version of this module
Core bool `mapstructure:"core"` // whether this module comes from core, meaning that no further dependencies will be added
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that we should keep this property for a couple of releases, telling users what to use instead.

jpkrohling
jpkrohling previously approved these changes Oct 14, 2021
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there! There's one typo, and one line needs to be removed. After this, I'll approve and merge. Thank you for your patience :-)

assert.Empty(t, cfg.Extensions[0].GoMod)
}

func TestDepricatedCore(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func TestDepricatedCore(t *testing.T) {
func TestDeprecatedCore(t *testing.T) {

Extensions: []Module{
{
Import: "go.opentelemetry.io/collector/receiver/jaegerreceiver",
GoMod: "go.opentelemetry.io/collector v0.36.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove this line? Previously, only the import and "core" were required, and this test would reflect that.

Fixes open-telemetry#15

Signed-off-by: yugo-horie <u5.horie@gmail.com>
@jpkrohling jpkrohling enabled auto-merge (squash) October 14, 2021 14:41
@jpkrohling jpkrohling merged commit a9f8085 into open-telemetry:main Oct 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Core property from Module type
2 participants