Skip to content

Commit ca386bd

Browse files
committed
chore: enforce TODO issue links in CI and update existing TODOs
This change introduces a new GitHub Actions workflow that enforces all TODO comments must reference a GitHub issue (e.g., TODO(#123): description). Plain TODOs without issue links will now fail CI. All existing TODOs in the codebase have been updated with links to their corresponding GitHub issues: - #493: Parallel resolution in Dotprompt.render() - #494: pending:true support for section helper in Go - #495: Empty response body behavior clarification - #496: Error handling for None resolver returns - #497: Python 3.12+ type syntax upgrade - #498: Schema caching in Dotprompt - #499: model_dump for config serialization - #500: Pagination implementation in Go DirStore - #501: Register helpers support in Go - #502: Options/local variables support in handlebarrz - #503: Smoke test improvements The TODO format requirement has been added to the coding guidelines. Fixes #493, #494, #495, #496, #497, #498, #499, #500, #501, #502, #503
1 parent cc918b1 commit ca386bd

File tree

14 files changed

+151
-18
lines changed

14 files changed

+151
-18
lines changed

.github/workflows/todo-check.yml

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
#
15+
# SPDX-License-Identifier: Apache-2.0
16+
17+
name: TODO Check
18+
19+
on:
20+
pull_request:
21+
branches: [main]
22+
push:
23+
branches: [main]
24+
25+
jobs:
26+
check-todos:
27+
name: Check TODOs have issue links
28+
runs-on: ubuntu-latest
29+
steps:
30+
- name: Checkout code
31+
uses: actions/checkout@v4
32+
33+
- name: Check for TODOs without issue links
34+
run: |
35+
#!/bin/bash
36+
set -euo pipefail
37+
38+
echo "🔍 Checking for TODOs without issue links..."
39+
echo ""
40+
41+
# Define patterns for valid TODOs with issue links
42+
# Valid formats:
43+
# // TODO(#123): description
44+
# // TODO(https://github.com/org/repo/issues/123): description
45+
# # TODO(#123): description
46+
# # TODO(https://...): description
47+
48+
# Find all TODO comments that DON'T have an issue reference
49+
# Pattern explanation:
50+
# - Match TODO comments without parentheses containing # or http
51+
# - Exclude: TODO(#123), TODO(https://...)
52+
53+
INVALID_TODOS=$(grep -rn \
54+
--include="*.py" \
55+
--include="*.ts" \
56+
--include="*.js" \
57+
--include="*.go" \
58+
--include="*.rs" \
59+
--include="*.java" \
60+
--include="*.kt" \
61+
--include="*.sh" \
62+
--include="*.yaml" \
63+
--include="*.yml" \
64+
-E '(#|//)\s*TODO[^(]|TODO\(\s*\)|TODO\([^#h][^)]*\)' \
65+
--exclude-dir=node_modules \
66+
--exclude-dir=.git \
67+
--exclude-dir=target \
68+
--exclude-dir=dist \
69+
--exclude-dir=build \
70+
--exclude-dir=.cache \
71+
--exclude-dir=bazel-* \
72+
--exclude-dir=vendor \
73+
--exclude-dir=site \
74+
--exclude-dir=.venv \
75+
--exclude-dir=venv \
76+
--exclude-dir=__pycache__ \
77+
. 2>/dev/null || true)
78+
79+
# Filter out documentation examples (in .md files showing TODO syntax)
80+
INVALID_TODOS=$(echo "$INVALID_TODOS" | grep -v "\.md:" || true)
81+
# Filter out this workflow file itself
82+
INVALID_TODOS=$(echo "$INVALID_TODOS" | grep -v "todo-check.yml" || true)
83+
84+
if [ -n "$INVALID_TODOS" ]; then
85+
echo "❌ Found TODOs without issue links:"
86+
echo ""
87+
echo "$INVALID_TODOS"
88+
echo ""
89+
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
90+
echo ""
91+
echo "📝 TODOs must reference a GitHub issue. Valid formats:"
92+
echo ""
93+
echo " Python/Bash/YAML:"
94+
echo " # TODO(#123): description"
95+
echo " # TODO(https://github.com/google/dotprompt/issues/123): description"
96+
echo ""
97+
echo " JavaScript/TypeScript/Java/Rust/Go:"
98+
echo " // TODO(#123): description"
99+
echo " // TODO(https://github.com/google/dotprompt/issues/123): description"
100+
echo ""
101+
echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━"
102+
echo ""
103+
echo "💡 To create an issue for your TODO:"
104+
echo " gh issue create --title 'Your TODO title' --body 'Description'"
105+
echo ""
106+
exit 1
107+
fi
108+
109+
echo "✅ All TODOs have proper issue links!"

docs/contributing/coding_guidelines.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,30 @@ for Generative AI. The codebase supports **Go**, **Python**, **JavaScript/TypeSc
1515
unless there is a compelling, documented reason.
1616
* **Check Licenses**: Run `./scripts/check_license` to ensure all files have proper
1717
license headers.
18+
* **TODO Format**: All TODOs must reference a GitHub issue. Plain `TODO:` comments
19+
without issue links are not allowed and will fail CI.
20+
21+
### TODO Format
22+
23+
TODOs must include an issue reference in parentheses:
24+
25+
```python
26+
# Python/Bash/YAML
27+
# TODO(#123): description of work to be done
28+
# TODO(https://github.com/google/dotprompt/issues/123): full URL also works
29+
```
30+
31+
```typescript
32+
// JavaScript/TypeScript/Java/Rust/Go
33+
// TODO(#123): description of work to be done
34+
// TODO(https://github.com/google/dotprompt/issues/123): full URL also works
35+
```
36+
37+
**Why?** Issue-linked TODOs ensure:
38+
- Work is tracked and not forgotten
39+
- Context is preserved for future developers
40+
- Progress can be measured
41+
- TODOs can be prioritized alongside other work
1842

1943
***
2044

go/dotprompt/dirstore.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (ds *DirStore) List(options ListPromptsOptions) (ListPromptsResult[PromptRe
142142
result := ListPromptsResult[PromptRef]{
143143
Items: prompts,
144144
}
145-
// TODO: meaningful cursor/limit implementation
145+
// TODO(#500): meaningful cursor/limit implementation
146146
// For now returns all as simple implementation
147147

148148
if options.Limit > 0 && len(result.Items) > options.Limit {

go/dotprompt/dotprompt.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ func (dp *Dotprompt) DefinePartial(name string, source string, tpl *raymond.Temp
159159
return nil
160160
}
161161

162-
// TODO: Add register helpers
162+
// TODO(#501): Add register helpers
163163
func (dp *Dotprompt) RegisterHelpers(tpl *raymond.Template) error {
164164
if dp.Helpers != nil {
165165
for key, helper := range dp.Helpers {

go/dotprompt/helper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ var templateHelpers = map[string]any{
3434
"unlessEquals": UnlessEquals,
3535
}
3636

37-
// TODO: Add pending: true for section helper
37+
// TODO(#494): Add pending: true for section helper
3838
// JSON serializes the given data to a JSON string with optional indentation.
3939
// Panics on serialization errors to match JavaScript's JSON.stringify fail-fast behavior.
4040
func JSON(serializable any, options *raymond.Options) raymond.SafeString {

go/dotprompt/parse_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ func TestExtractFrontmatterAndBody(t *testing.T) {
420420
})
421421

422422
t.Run("should return empty strings when there is no frontmatter marker", func(t *testing.T) {
423-
// TODO: May be change this behavior to return a matching body when
423+
// TODO(#495): May be change this behavior to return a matching body when
424424
// there is no frontmatter marker and we have a body. This may need to
425425
// be done across all the runtimes.
426426
inputStr := "Hello World"

js/src/dotprompt.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ export class Dotprompt {
298298
out = outWithoutTemplate as PromptMetadata<ModelConfig>;
299299

300300
out = removeUndefinedFields(out);
301-
// TODO: Can this be done concurrently?
301+
// TODO(#493): Can this be done concurrently?
302302
out = await this.resolveTools(out);
303303
out = await this.renderPicoschema(out);
304304

js/src/parse.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -856,7 +856,7 @@ Template content`;
856856
});
857857

858858
it('should handle document with empty frontmatter', () => {
859-
// TODO: Check whether this is the correct behavior.
859+
// TODO(#495): Check whether this is the correct behavior.
860860
const source = '---\n\n---\nJust template content';
861861
const result = parseDocument(source);
862862
expect(result).toMatchObject({

python/dotpromptz/src/dotpromptz/dotprompt.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ async def _resolve_metadata(
379379
delattr(out, 'template')
380380

381381
out = remove_undefined_fields(out)
382-
# TODO: can this be done concurrently?
382+
# TODO(#493): can this be done concurrently?
383383
out = await self._resolve_tools(out)
384384
out = await self._render_picoschema(out)
385385
return out
@@ -417,10 +417,10 @@ async def _process_output_schema(schema_to_process: Any) -> None:
417417

418418
async with anyio.create_task_group() as tg:
419419
if needs_input_processing and meta.input is not None:
420-
# TODO: use meta.input.model_dump(exclude_none=True)?
420+
# TODO(#499): use meta.input.model_dump(exclude_none=True)?
421421
tg.start_soon(_process_input_schema, meta.input.schema)
422422
if needs_output_processing and meta.output is not None:
423-
# TODO: use meta.output.model_dump(exclude_none=True)?
423+
# TODO(#499): use meta.output.model_dump(exclude_none=True)?
424424
tg.start_soon(_process_output_schema, meta.output.schema)
425425

426426
return new_meta
@@ -440,7 +440,7 @@ async def _wrapped_schema_resolver(self, name: str) -> JsonSchema | None:
440440
if self._schema_resolver is None:
441441
return None
442442

443-
# TODO: Should we cache the resolved schema in self._schemas?
443+
# TODO(#498): Should we cache the resolved schema in self._schemas?
444444
return await resolve_json_schema(name, self._schema_resolver)
445445

446446
async def _resolve_tools(self, metadata: PromptMetadata[ModelConfigT]) -> PromptMetadata[ModelConfigT]:

python/dotpromptz/src/dotpromptz/parse.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ def parse_document(source: str) -> ParsedPrompt[T]:
274274
template=body.strip(),
275275
)
276276
except Exception as e:
277-
# TODO: Should this be an error?
277+
# TODO(#496): Should this be an error?
278278
print(f'Dotprompt: Error parsing YAML frontmatter: {e}')
279279
# Return a basic ParsedPrompt with just the template
280280
return ParsedPrompt(

0 commit comments

Comments
 (0)