fix(#1768): remove stale .adoc files before doc generation#1769
fix(#1768): remove stale .adoc files before doc generation#1769fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
GenerateAsciidoc() only created/overwrote .adoc files for currently existing packages but never removed files for deleted packages. This caused stale documentation to persist in the repo and published docs. Add removeAdocFiles() helper that clears all .adoc files from the packages/ directory before regenerating. Since everything in that directory is generated content, this is safe and simpler than a marker-based approach. Add unit tests covering removal of .adoc files, preservation of non-.adoc files and subdirectories, and graceful handling of nonexistent or empty directories. Closes #1768
|
🤖 Finished Review · ✅ Success · Started 4:11 PM UTC · Completed 4:21 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Review of PR #1769Verdict: Approve ✅ Clean, well-scoped fix that precisely implements the approach suggested in #1768. The new What was reviewed
Findings[low] The function uses [low] Pre-existing nil-dereference risk —
Neither finding blocks this PR. Labels: PR fixes a bug in documentation generation tooling |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| func removeAdocFiles(dir string) error { | ||
| entries, err := os.ReadDir(dir) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { |
There was a problem hiding this comment.
[low] best-practice
Uses os.IsNotExist(err) instead of the idiomatic errors.Is(err, fs.ErrNotExist). While both work correctly here (os.ReadDir returns an unwrapped *os.PathError), the errors.Is form handles wrapped errors and is the Go 1.13+ convention. The io/fs package is already imported in this file.
Suggested fix: Replace if os.IsNotExist(err) with if errors.Is(err, fs.ErrNotExist) and add "errors" to the import block.
There was a problem hiding this comment.
Will fullsend the coder notice this suggestion made by fullsend the reviewer, and make the suggested change?
| if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".adoc") { | ||
| if err := os.Remove(filepath.Join(dir, entry.Name())); err != nil { | ||
| return fmt.Errorf("removing file %q: %w", entry.Name(), err) | ||
| } |
There was a problem hiding this comment.
[low] nil-dereference
Pre-existing bug (not introduced by this PR): for _, p := range *d.Packages panics when d.Packages is nil, which occurs when SetAnnotations finds zero packages for a doc category. While all five categories currently have packages, adding a nil guard would prevent a crash if a category ever becomes empty.
Suggested fix: Wrap the range loop in if d.Packages != nil { ... }.
fullsend-ai-review
Bot
added
ready-for-merge
1 participant
GenerateAsciidoc() only created/overwrote .adoc files for currently existing packages but never removed files for deleted packages. This caused stale documentation to persist in the repo and published docs.
Add removeAdocFiles() helper that clears all .adoc files from the packages/ directory before regenerating. Since everything in that directory is generated content, this is safe and simpler than a marker-based approach.
Add unit tests covering removal of .adoc files, preservation of non-.adoc files and subdirectories, and graceful handling of nonexistent or empty directories.
Closes #1768
Post-script verification
agent/1768-remove-stale-docs)621fc39ae43ed1298a37c971f3483c59c8ef5a60..HEAD)