Skip to content

Commit 3050efc

Browse files
fix: address review feedback on PR #3383
- Eliminate marker duplication: remove hardcoded marker from all 4 templates (cli.tmpl, rego.tmpl, builtins.tmpl, task.tmpl) and write it programmatically via fmt.Fprintln before template execution. The GeneratedMarker constant is now the single source of truth, passed as a parameter to each generator function. - Fix premature append in cli.go: move generated-path append after successful commandTemplate.Execute so failed writes do not appear in the generated list. - Add logging on abandoned file removal: emit a message to stderr after each successful os.Remove in removeAbandonedPages. - Add test for os.ReadDir error path: verify removeAbandonedPages returns an error when the pages directory does not exist. Addresses review feedback on #3383
1 parent 28f46d2 commit 3050efc

9 files changed

Lines changed: 47 additions & 20 deletions

File tree

‎internal/documentation/asciidoc/asciidoc.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,19 @@ const GeneratedMarker = "// Generated by conforma CLI documentation tooling. DO
3535
func GenerateAsciidoc(module string) error {
3636
var generated []string
3737

38-
cliGenerated, err := cli.GenerateCommandLineDocumentation(module)
38+
cliGenerated, err := cli.GenerateCommandLineDocumentation(module, GeneratedMarker)
3939
if err != nil {
4040
return err
4141
}
4242
generated = append(generated, cliGenerated...)
4343

44-
regoGenerated, err := rego.GenerateRegoReference(module)
44+
regoGenerated, err := rego.GenerateRegoReference(module, GeneratedMarker)
4545
if err != nil {
4646
return err
4747
}
4848
generated = append(generated, regoGenerated...)
4949

50-
tektonGenerated, err := tekton.GenerateTektonDocumentation(module)
50+
tektonGenerated, err := tekton.GenerateTektonDocumentation(module, GeneratedMarker)
5151
if err != nil {
5252
return err
5353
}
@@ -98,6 +98,7 @@ func removeAbandonedPages(module string, generated []string) error {
9898
if err := os.Remove(path); err != nil {
9999
return fmt.Errorf("removing abandoned file %q: %w", path, err)
100100
}
101+
fmt.Fprintf(os.Stderr, "Removed abandoned generated file: %s\n", path)
101102
}
102103

103104
return nil

‎internal/documentation/asciidoc/asciidoc_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,14 @@ func TestRemoveAbandonedPages(t *testing.T) {
117117
err := removeAbandonedPages(module, []string{})
118118
require.NoError(t, err)
119119
})
120+
121+
t.Run("returns error for non-existent directory", func(t *testing.T) {
122+
module := filepath.Join(t.TempDir(), "nonexistent")
123+
124+
err := removeAbandonedPages(module, []string{})
125+
require.Error(t, err)
126+
assert.Contains(t, err.Error(), "reading pages directory")
127+
})
120128
}
121129

122130
func TestHasGeneratedMarker(t *testing.T) {

‎internal/documentation/asciidoc/cli/cli.go‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ func init() {
5959
}).Parse(cliNavTemplateText))
6060
}
6161

62-
func GenerateCommandLineDocumentation(module string) ([]string, error) {
62+
func GenerateCommandLineDocumentation(module, marker string) ([]string, error) {
6363
var generated []string
64-
if err := generateCommandReference(cmd.RootCmd, module, &generated); err != nil {
64+
if err := generateCommandReference(cmd.RootCmd, module, marker, &generated); err != nil {
6565
return nil, err
6666
}
6767

@@ -72,13 +72,13 @@ func GenerateCommandLineDocumentation(module string) ([]string, error) {
7272
return generated, nil
7373
}
7474

75-
func generateCommandReference(cmd *cobra.Command, module string, generated *[]string) error {
75+
func generateCommandReference(cmd *cobra.Command, module, marker string, generated *[]string) error {
7676
for _, c := range cmd.Commands() {
7777
if !c.IsAvailableCommand() || c.IsAdditionalHelpTopicCommand() {
7878
continue
7979
}
8080

81-
if err := generateCommandReference(c, module, generated); err != nil {
81+
if err := generateCommandReference(c, module, marker, generated); err != nil {
8282
return fmt.Errorf("generating Asciidoc for command %q: %w", c.Name(), err)
8383
}
8484
}
@@ -93,9 +93,17 @@ func generateCommandReference(cmd *cobra.Command, module string, generated *[]st
9393
}
9494
defer f.Close()
9595

96+
if _, err := fmt.Fprintln(f, marker); err != nil {
97+
return fmt.Errorf("writing marker to %q: %w", docpath, err)
98+
}
99+
100+
if err := commandTemplate.Execute(f, cmd); err != nil {
101+
return err
102+
}
103+
96104
*generated = append(*generated, docpath)
97105

98-
return commandTemplate.Execute(f, cmd)
106+
return nil
99107
}
100108

101109
func generateCommandReferenceNav(root *cobra.Command, module string) error {

‎internal/documentation/asciidoc/cli/cli.tmpl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Generated by conforma CLI documentation tooling. DO NOT EDIT.
21
= {{ .CommandPath }}
32

43
{{ .Short }}

‎internal/documentation/asciidoc/rego/builtins.tmpl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Generated by conforma CLI documentation tooling. DO NOT EDIT.
21
= ec rego functions reference documentation
32

43
The EC CLI provides custom rego builtin functions in addition to the

‎internal/documentation/asciidoc/rego/rego.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ func init() {
8383
builtins = findBuiltins()
8484
}
8585

86-
func GenerateRegoReference(module string) ([]string, error) {
87-
generated, err := generateRegoReference(module)
86+
func GenerateRegoReference(module, marker string) ([]string, error) {
87+
generated, err := generateRegoReference(module, marker)
8888
if err != nil {
8989
return nil, err
9090
}
@@ -93,7 +93,7 @@ func GenerateRegoReference(module string) ([]string, error) {
9393
return nil, err
9494
}
9595

96-
builtinsPath, err := generateRegoBuiltins(module)
96+
builtinsPath, err := generateRegoBuiltins(module, marker)
9797
if err != nil {
9898
return nil, err
9999
}
@@ -117,7 +117,7 @@ func findBuiltins() []*ast.Builtin {
117117
return builtins
118118
}
119119

120-
func generateRegoReference(module string) ([]string, error) {
120+
func generateRegoReference(module, marker string) ([]string, error) {
121121
var generated []string
122122
for _, b := range builtins {
123123
docpath := filepath.Join(module, "pages", strings.ReplaceAll(b.Name, ".", "_")+".adoc")
@@ -126,6 +126,11 @@ func generateRegoReference(module string) ([]string, error) {
126126
return nil, fmt.Errorf("creating file %q: %w", docpath, err)
127127
}
128128

129+
if _, err := fmt.Fprintln(f, marker); err != nil {
130+
f.Close()
131+
return nil, fmt.Errorf("writing marker to %q: %w", docpath, err)
132+
}
133+
129134
if err := regoTemplate.Execute(f, b); err != nil {
130135
f.Close()
131136
return nil, err
@@ -150,13 +155,17 @@ func generateRegoReferenceNav(module string) error {
150155
return regoNavTemplate.Execute(f, builtins)
151156
}
152157

153-
func generateRegoBuiltins(module string) (string, error) {
158+
func generateRegoBuiltins(module, marker string) (string, error) {
154159
builtinsPath := filepath.Join(module, "pages", "rego_builtins.adoc")
155160
f, err := os.Create(builtinsPath)
156161
if err != nil {
157162
return "", fmt.Errorf("creating file %q: %w", builtinsPath, err)
158163
}
159164
defer f.Close()
160165

166+
if _, err := fmt.Fprintln(f, marker); err != nil {
167+
return "", fmt.Errorf("writing marker to %q: %w", builtinsPath, err)
168+
}
169+
161170
return builtinsPath, regoBuiltinsTemplate.Execute(f, builtins)
162171
}

‎internal/documentation/asciidoc/rego/rego.tmpl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
{{- end }}{{- /* range */ -}}
4040
{{- end -}}{{- /* "properties" */ -}}
4141

42-
// Generated by conforma CLI documentation tooling. DO NOT EDIT.
4342
= {{ .Name }}
4443

4544
{{ .Description }}

‎internal/documentation/asciidoc/tekton/task.tmpl‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
// Generated by conforma CLI documentation tooling. DO NOT EDIT.
21
= {{ .Name}}
32

43
Version: {{ index .Labels "app.kubernetes.io/version" }}

‎internal/documentation/asciidoc/tekton/tekton.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ func init() {
7373
}
7474
}
7575

76-
func GenerateTektonDocumentation(module string) ([]string, error) {
77-
generated, err := generateTektonReference(module)
76+
func GenerateTektonDocumentation(module, marker string) ([]string, error) {
77+
generated, err := generateTektonReference(module, marker)
7878
if err != nil {
7979
return nil, err
8080
}
@@ -86,7 +86,7 @@ func GenerateTektonDocumentation(module string) ([]string, error) {
8686
return generated, nil
8787
}
8888

89-
func generateTektonReference(module string) ([]string, error) {
89+
func generateTektonReference(module, marker string) ([]string, error) {
9090
var generated []string
9191
for _, task := range tasks {
9292
docpath := filepath.Join(module, "pages", task.Name+".adoc")
@@ -95,6 +95,11 @@ func generateTektonReference(module string) ([]string, error) {
9595
return nil, fmt.Errorf("creating file %q: %w", docpath, err)
9696
}
9797

98+
if _, err := fmt.Fprintln(f, marker); err != nil {
99+
f.Close()
100+
return nil, fmt.Errorf("writing marker to %q: %w", docpath, err)
101+
}
102+
98103
if err := tektonTaskTemplate.Execute(f, task); err != nil {
99104
f.Close()
100105
return nil, err

0 commit comments

Comments
 (0)