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

fix: Go build remove unused imports #47

Merged
merged 5 commits into from
Nov 14, 2023
Merged

Conversation

ocobles
Copy link
Contributor

@ocobles ocobles commented Nov 13, 2023

When generating the go sdk it includes some dependencies that are not used, this produces errors during the installation of the package. I added goimports in the build_go target to remove all unused imports

go install golang.org/x/tools/cmd/goimports@latest
cd sdk && goimports -w . && go list "$$(grep -e "^module" go.mod | cut -d ' ' -f 2)/go/..." | xargs go build

There are also some updates in go.mod/go.sum files after running go work sync, and goreleaser file updated to run clean cache

Signed-off-by: ocobleseqx <[email protected]>
Signed-off-by: ocobleseqx <[email protected]>
Copy link

Does the PR have any schema changes?

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Maintainer note: consult the runbook for dealing with any breaking changes.

@ocobles ocobles changed the title Go build remove unused imports fix: Go build remove unused imports Nov 14, 2023
@@ -8,6 +8,7 @@ import (
"reflect"

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 imagine goimports would have removed this whitespace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it was goimports

@displague
Copy link
Member

Have you tried https://go.dev/ref/mod#go-mod-tidy?

@displague
Copy link
Member

cd sdk && goimports -w . && go list "$$(grep -e "^module" go.mod | cut -d ' ' -f 2)/go/..." | xargs go build

This seems dangerous compared to go mod tidy. Is it being used elsewhere?

We may be picking up development/build runtime dependencies, and if those are creating a problem we could put them in a separate go.mod.

@ocobles
Copy link
Contributor Author

ocobles commented Nov 14, 2023

I didn't see go mod tidy removing unused imports from code, AFAIK it is just the other way around, it removes the dependency from go.mod if it is not referenced in any file

@ocobles
Copy link
Contributor Author

ocobles commented Nov 14, 2023

Just to clarify, I've opened this PR because I've verified that the latest Go SDK package generated isn't working. It seems that Pulumi internally executes 'go build' during 'pulumi up' and returns below error

image

@displague displague merged commit 2cfc8f9 into main Nov 14, 2023
14 checks passed
@ctreatma ctreatma deleted the go-build-remove-unused-imports branch August 21, 2024 14:52
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