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 integration go.mod #231

Merged
merged 5 commits into from
Apr 26, 2024
Merged

Remove integration go.mod #231

merged 5 commits into from
Apr 26, 2024

Conversation

blampe
Copy link
Contributor

@blampe blampe commented Apr 25, 2024

A module is a collection of packages that are released, versioned, and distributed together.

There's no need to version this subdirectory separately from the repo's root module because they are tightly coupled.

  • The last time we remembered to tag this module was integration/v0.10.0, which is old enough to have incompatibilities with main. A new tag integration/v0.16.0 is needed for compatibility with v0.16.0, so these are not versioned separately.

  • The invalid github.com/pulumi/pulumi-go-provider v0.0.0-00010101000000-000000000000 constraint makes the module un-importable on its own, it must be imported alongside github.com/pulumi/pulumi-go-provider -- so we're already distributing these together.

go: downloading github.com/pulumi/pulumi-go-provider v0.0.0-00010101000000-000000000000
go: github.com/blampe/gomodexample imports
github.com/pulumi/pulumi-go-provider/integration imports
github.com/pulumi/pulumi-go-provider: github.com/pulumi/pulumi-go-provider@v0.0.0-00010101000000-000000000000: invalid version: unknown revision 000000000000

  • The replace github.com/pulumi/pulumi-go-provider => ../ directive is another smell that this module is actually just a member of the parent module, since it wants the parent module's code at the same commit.

In other words, we don't release/version/distribute these things separately, so there's no need to treat them as separate modules. We can make life easier for ourselves by removing this go.mod.

The only impact this will have on downstream is that they will need to remove github.com/pulumi/pulumi-go-provider/integration from go.mod, since it will become an ambiguous import alongside github.com/pulumi/pulumi-go-provider.

@blampe blampe requested a review from iwahbe April 25, 2024 20:23
Copy link
Member

@iwahbe iwahbe left a comment

Choose a reason for hiding this comment

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

I'm vaguely uncomfortable with sharing dev dependencies with prod dependencies, but Go is not.

integration/integration.go Outdated Show resolved Hide resolved
Co-authored-by: Ian Wahbe <ian@wahbe.com>
@blampe
Copy link
Contributor Author

blampe commented Apr 26, 2024

I'm vaguely uncomfortable with sharing dev dependencies with prod dependencies, but Go is not.

At the end of the day everything is just a package, and yeah it's normal to group together test helpers and mocks into something like httptest.

For what it's worth, the dependencies you take on by importing this from the "pulumi-go-provider" module versus importing it from a separate "pulumi-go-provider/integration" module are identical. In other words, in both cases you only take on the subset of dependencies that integration/go.mod currently shows.

I think this has bitten us in pu/pu because our helpers (pkg/v3/testing/integration in particular) are heavy in the sense that they end up pulling in a large chunk of business logic and production dependencies.

@blampe blampe merged commit 72606b1 into main Apr 26, 2024
6 checks passed
@blampe blampe deleted the blampe/integration-mod branch April 26, 2024 16:30
thomas11 added a commit to pulumi/pulumi-command that referenced this pull request May 17, 2024
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.

2 participants