Commit f6d8d71
committed
refactor: address code review improvements
Implemented all suggested improvements from code review:
**1. Test Robustness (Timing Assertions)**
- Added comments to timing-based test assertions noting potential CI flakiness
- Documented generous timeouts (30s, 10min) that account for CI overhead
- Tests: imageUrlExpiration.test.ts (lines 302-309, 424-428, 467-471)
**2. Documentation: Phase 3 Status**
- Clearly marked Phase 3 as "OPTIONAL/FUTURE WORK" in spec
- Added status section: "NOT IMPLEMENTED - Future enhancement"
- Documented when Phase 3 should be implemented (only if Phase 1/2 insufficient)
- Made it clear Phases 1 & 2 solve Issue #94
- Spec: IMAGE_URL_EXPIRATION_SPEC.md (lines 334-355)
**3. Type Safety Improvement**
- Changed isExpiredUrlError parameter from `any` to `unknown`
- Added ErrorWithResponse interface for proper type checking
- Added type guard: checks error is object before casting
- Better type safety while maintaining flexibility
- File: imageProcessing.ts (lines 29-46)
**4. Edge Case Test: Callouts with Images**
- Added test for callouts containing images after reordering
- Verifies both image download and callout transformation work correctly
- Ensures images are processed before callouts (order dependency)
- Test: imageUrlExpiration.test.ts (lines 530-556)
**Test Results:**
✅ All 17 expired URL detection tests pass
✅ All 9 download image tests pass
✅ New edge case test passes
✅ No new type errors introduced
**Risk Assessment:**
- Low risk: improvements don't change logic, only add clarity/safety
- All existing tests continue to pass
- Type safety improvement prevents potential runtime errors
- Documentation improvements prevent future confusion
Related to code review feedback on PR for Issue #941 parent 8e8e5e9 commit f6d8d71
File tree
3 files changed
+140
-41
lines changed- scripts/notion-fetch
- __tests__
3 files changed
+140
-41
lines changed
0 commit comments