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

Go: Remove go.work file #16738

Merged
merged 4 commits into from
Jun 13, 2024
Merged

Go: Remove go.work file #16738

merged 4 commits into from
Jun 13, 2024

Conversation

mbg
Copy link
Member

@mbg mbg commented Jun 12, 2024

This (once again) removes the go.work file from the repository. Our main motivation for having it was that gopls (which is used by the Go extension for VSCode) seemed to require having one for some time, but now:

@mbg mbg self-assigned this Jun 12, 2024
@mbg mbg requested a review from a team as a code owner June 12, 2024 14:12
@github-actions github-actions bot added the Go label Jun 12, 2024
@owen-mc
Copy link
Contributor

owen-mc commented Jun 12, 2024

We should check with @redsun82 whether any changes are needed to the bazel build system. For example, I just came across the fact that go/gen.py runs go work vendor to update the vendor directory (on line 51).

@mbg
Copy link
Member Author

mbg commented Jun 12, 2024

Yep it seems like that breaks now in CI

@redsun82
Copy link
Contributor

I think that was the way I found to vendor dependencies without using -m vendor in compilation when the go.work file was around, but what @mbg is doing with go mod vendor seems the way to go (there's just a small diff to apply removing the workspace comment`)

@mbg
Copy link
Member Author

mbg commented Jun 12, 2024

@owen-mc looks fine now

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

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

Seems to work on my machine

@mbg mbg merged commit 1834a39 into main Jun 13, 2024
13 checks passed
@mbg mbg deleted the mbg/go/remove-go-work branch June 13, 2024 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants