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

feat: support for go.work #2849

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

mdelapenya
Copy link
Member

@mdelapenya mdelapenya commented Oct 25, 2024

What does this PR do?

This PR adds support for creating a go.work file and managing it when new modules are created using the modulegen tool.

During this process, we had to rework a bit this code generation tool, using the modfile package to handle the workfile programatically.
Besides that, we updated the existing tests to check the work files are properly generated, also moving tests to the right package and even
removing duplicated tests (e.g. for validation).

Finally, we are improving the error messages, following recent patterns we are adopting in the library.

Important to notice the amount of lines in the changeset comes from running go work sync, which updated all the dependencies.

Why is it important?

Now the go toolchain will be able to handle the core module and its submodules in a proper manner, so it would be much easier to setup the IDE when working on the project.
In the past, we generated code for VSCode and handle all the submodules, now it would be possible to just open the root dir, and because of the existence of the go.work file,
the IDE would be able to follow all links across the entire code base.

Related issues

@mdelapenya mdelapenya requested a review from a team as a code owner October 25, 2024 13:06
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 25, 2024
@mdelapenya mdelapenya self-assigned this Oct 25, 2024
Copy link

netlify bot commented Oct 25, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit cbfdea7
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/6720b6999366710008b34bb6
😎 Deploy Preview https://deploy-preview-2849--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

* main:
  chore: use require.(No)Error(t,err) instead of t.Fatal(err) (testcontainers#2851)
@@ -14,7 +14,5 @@ TEST-*.xml

tcvenv

**/go.work
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: do we want it committed or part of a setup process? Typically its not standard to add this, but I wonder if this is a good example for it?

modulegen/context_test.go Outdated Show resolved Hide resolved
modulegen/internal/context/context.go Outdated Show resolved Hide resolved
modulegen/internal/context/types_test.go Outdated Show resolved Hide resolved
modulegen/internal/tools/exec.go Show resolved Hide resolved
modulegen/internal/workfile/main.go Outdated Show resolved Hide resolved
return Write(rootGoWorkFilePath, rootGoWork)
}

func newWorkFile(goWork *modfile.WorkFile, examples []string, modules []string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: is the intent to exclude other modules like test, and wait? If not using a find for go.mod might be simpler.

Copy link
Member Author

Choose a reason for hiding this comment

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

but wait is a package, not a module, right?

modulegen/internal/workfile/reader.go Show resolved Hide resolved
modulegen/internal/workfile/writer.go Show resolved Hide resolved
modules/localstack/v2/s3_test.go Show resolved Hide resolved
@mdelapenya
Copy link
Member Author

@mmorel-35 pinging you for a review (if possible), as you worked in the modulegen a lot 🙏

modulegen/internal/main.go Show resolved Hide resolved
modules/localstack/v2/s3_test.go Show resolved Hide resolved
modulegen/internal/context/types_test.go Show resolved Hide resolved
modulegen/internal/tools/exec.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Changes that do not impact the existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: modulegen: print output from failed commands
3 participants