Conversation
747050a to
63e851c
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for embedding template files using Go's go:embed directive, enabling the OpenAPI generator to bundle templates directly into the binary. This eliminates the need for external template directories and simplifies distribution as a single binary.
Changes:
- Added embedded filesystem support using
go:embedfor typescript-fetch templates - Introduced fallback mechanism: uses filesystem templates when
--template-diris specified, otherwise uses embedded templates - Changed file permissions from 0644 to 0600 for generated files (security improvement)
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/embed.go | New file defining embedded filesystem containing typescript-fetch template files |
| internal/template/embedded.go | New file implementing engine creation and partial loading from embedded filesystem |
| internal/template/mustache.go | Added fsys field and dual-mode rendering supporting both filesystem and embedded templates; changed file permissions to 0600 |
| cmd/openapi-generator/main.go | Updated template loading logic to fallback to embedded templates when no template directory is found; applied file permission changes |
| internal/parser/openapi.go | Refactored if-else to switch statement for number format handling |
| internal/generator/typescript/base.go | Simplified condition logic in Camelize function using De Morgan's laws |
| .github/workflows/quality.yml | Renamed workflow from "Go quality checks" to "Quality checks" |
| .github/workflows/coverage.yml | Renamed workflow from "Go coverage report" to "Coverage report" |
| name := strings.TrimPrefix(path, e.TemplateDir+"/") | ||
| name = strings.TrimSuffix(name, ".mustache") |
There was a problem hiding this comment.
The path construction in LoadPartialsFromFS uses simple string concatenation with "/" but doesn't handle potential edge cases where partial names might not normalize correctly. The original LoadPartials uses filepath.Rel to compute the relative path, then normalizes separators with strings.ReplaceAll. For consistency and to handle potential subdirectories within the template directory correctly, consider using a similar normalization approach:
Instead of:
name := strings.TrimPrefix(path, e.TemplateDir+"/")
Consider handling cases where the template directory path might not have a trailing separator, and ensure proper handling of nested partials. For example, if there are subdirectories in typescript-fetch, the partial name computation should be consistent with the filesystem-based version.
No description provided.