-
-
Notifications
You must be signed in to change notification settings - Fork 523
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(modulegen): print out VSCode workspace file if needed #1549
feat(modulegen): print out VSCode workspace file if needed #1549
Conversation
This reverts commit 2d442d2.
Default is false
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@mmorel-35 could you please take a look? |
modulegen/internal/template/main.go
Outdated
func Generate(t *template.Template, exampleFilePath string, name string, data any) error { | ||
err := os.MkdirAll(filepath.Dir(exampleFilePath), 0o755) | ||
if err != nil { | ||
return err | ||
} | ||
exampleFile, _ := os.Create(exampleFilePath) | ||
defer exampleFile.Close() | ||
|
||
err = t.ExecuteTemplate(exampleFile, name, data) | ||
func Generate(t *template.Template, wr io.Writer, name string, data any) error { | ||
err := t.ExecuteTemplate(wr, name, data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of having Two functions instead
func GenerateFile(t *template.Template, exampleFilePath string, name string, data any) error {
And
func Generate(t *template.Template, wr io.Writer, name string, data any) error {
This way you keep inside GenerateFile
the logic of creation of directories and file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea. Let me rework on this PR
* main: modulegen: create internal/module and internal/modfile (testcontainers#1539)
General question. |
I did not want to couple it with VSCode, but you are right: we already have a devcontainer file. Do you think generating it in the root folder with a hidden name is fine? Something like |
Another question.
|
What are the practices recommended by vscode. From what I could see they usually define a dedicated folder in the repository. Edit: |
At some point I'd like to have one command per file that is not for the module itself, said in other words: a shared file: ci.yml, mkdocs, dependabot, vscode workspace... So I want to be able to refresh them and put it on sync |
@mmorel-35 I'm thinking about the content of the VSCode file:
The |
Would it work if the path was relative to the file ? You can even complete the gitignore do it's never committed. |
We can keep it under the |
The file is useful for anyone who wants to develop on this project. But as you said it, depending on the platform and the users setup the file path for the modules will be different. So modulegen can provide a way to generate the file but needs to ensure that you don't receive the config file of somebody else . |
I think I have it: thanks to the existing building blocks, I can get the root dir, retrieve its absolute file path and pass it to the template, for all modules. Using If you like the implementation, I'll update the description to reflect the discussion in here |
modulegen/main_test.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why removing those tests ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because they were too coupled to the implementation, and having to update them with every change "smelled to me". In fact, there are many of this type of tests in the module generator TBH
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It LGTM !
You can update the description with what we discussed.
* main: modulegen: generate md file inside internal/mkdocs (testcontainers#1543)
Kudos, SonarCloud Quality Gate passed!
|
Updated! Thanks for your review and guidance through this refactor, lots of learning! |
* main: (32 commits) fix: remove wrong example from workspace (testcontainers#1556) chore(deps): bump the all group in /modules/localstack with 1 update (testcontainers#1552) modulegen: generate code-workspace with json marshal (testcontainers#1551) chore(deps): bump the all group in /modules/compose with 2 updates (testcontainers#1553) feat: add mariadb module (testcontainers#1548) feat(modulegen): print out VSCode workspace file if needed (testcontainers#1549) modulegen: generate md file inside internal/mkdocs (testcontainers#1543) modulegen: create internal/module and internal/modfile (testcontainers#1539) [Enhancement]: add ability to set repo:tag for ContainerRequest FromDockerfile (testcontainers#1508) Fix module generator for examples (testcontainers#1545) Update pulsar.md (testcontainers#1542) modulegen: create internal/make (testcontainers#1537) chore: fix workflow (testcontainers#1538) chore(deps): bump the all group in /examples/cockroachdb with 1 update (testcontainers#1522) chore(deps): bump the all group in /examples/bigtable with 1 update (testcontainers#1534) chore(deps): bump the all group in /modules/localstack with 4 updates (testcontainers#1535) chore(deps): bump the all group in /modules/k3s with 2 updates (testcontainers#1526) chore(deps): bump the all group in /examples/spanner with 2 updates (testcontainers#1532) chore(deps): bump the all group in /examples/firestore with 1 update (testcontainers#1523) chore(deps): bump the all group in /modules/redis with 1 update (testcontainers#1524) ...
What does this PR do?
Following the recent refactors, it's now very easy to extend the module generator to generate more things. In this PR we are generating the VSCode workspace file, but instead of generating a file that could live whenever the user wants, it will generate the workspace file into the
.vscode
directory of the repository. This dir is ignored by git, so it won't impact in other users when the file is generated including absolute paths.To achieve this, we have adapted how our internal templating engine works: it accepted a string with the path where to write the file, in this PR we are adding a new function to also accept an
io.Writer
reference: Generate will use the Writer, and GenerateFile will use the string file, calling back to Generate once a File is created (os.File implements io.Writer).Before this PR, there were a few calls to this generation, so we have updated the calls to use the new
GenerateFile
function.The generation of the VSCode template happens at the end of the generation.
Why is it important?
I see myself many times refreshing the VSCode workspace to see all modules in one single VSCode project, and I do this manually. With the current refactors (big kudos to @mmorel-35!!!) now it's easy to extend the generator with a few lines of code, and this improvement was very cheap in time and code.