8.6 KiB
Code Cleanup Report - SkyWatch
Generated: Phase 4 of Feature Implementation
Summary
This report identifies areas for potential cleanup in the SkyWatch codebase after implementing 11 new features.
1. Console Statements (Debugging)
Recommendation: Remove or wrap in DEBUG flag
Total Found: 100+ console.log/error statements across the codebase
Server Files (server/):
-
server/index.js: ~40 console statements
- Lines 74, 80: Timezone offset debugging
- Lines 728, 736, 741, 744, 772, 775, 778, 799, 801, 810: History API debugging
- Lines 864, 869, 872, 900: SLA query debugging
- Lines 1483-1484: Server startup messages (KEEP THESE)
- Recommendation: Remove timezone/query debug logs, keep server startup and critical errors
-
server/database.js: ~20 console statements
- Lines 143, 146, 176, 179, 204, 207, 213, 216, 222, 225, 231, 234, 240, 243: Migration confirmations
- Recommendation: Keep these for database migration tracking
-
server/monitorService.js: 1 console.error (line 686)
- Recommendation: Keep error logging
-
server/notificationService.js: ~10 console.log statements
- Lines 54, 60, 66, 81, 85, 87, 172: Alert logic debugging
- Recommendation: Keep critical alerts, remove verbose debugging
Client Files (client/src/):
-
client/src/App.js: ~20 console.error statements
- All are error handlers in catch blocks
- Recommendation: Keep all - useful for user troubleshooting
-
client/src/History.js: 5 console statements
- Lines 37, 67: API response debugging
- Line 131: Chart data debugging
- Recommendation: Remove debug logs (37, 67, 131), keep error handling (72, 60)
-
client/src/SettingsWizard.js: 2 console.error
- Recommendation: Keep error logging
-
client/src/ErrorBoundary.js: 1 console.error (line 16)
- Recommendation: Keep error logging
-
client/src/SLA.js: 1 console.error (line 28)
- Recommendation: Keep error logging
Action Items:
REMOVE (Non-critical debug logs):
- server/index.js: Lines 74, 80, 728, 736, 741, 744, 772, 775, 778, 799, 801, 810, 864, 869, 872, 900
- server/notificationService.js: Lines 81, 85, 87 (verbose email debugging)
- client/src/History.js: Lines 37, 67, 131
KEEP (Critical logs):
- All console.error statements in error handlers
- server/index.js: Lines 1483-1484 (server startup)
- server/database.js: All migration logs
- server/notificationService.js: Lines 54, 60, 66, 172 (alert skip reasons)
2. Commented Code
Analysis: Minimal commented code found
- client/src/History.js line 129: Helpful comment explaining server-side averaging
- Build files contain source maps (expected)
Recommendation: No action needed - comments are descriptive, not dead code
3. Unused Dependencies
Server (package.json)
✅ All dependencies are used:
- express: ✓ Used in server/index.js
- cors: ✓ Used in server/index.js
- better-sqlite3: ✓ Used in server/database.js
- node-cron: ✓ Used in server/scheduler.js
- axios: ✓ Used in server/monitorService.js
- nodemailer: ✓ Used in server/notificationService.js
- dotenv: ✓ Used in server/index.js
- ping: ✓ Used in server/monitorService.js (line 252)
- ws: ✓ Used in server/index.js (WebSocket server)
Client (client/package.json)
⚠️ Potentially unused dependencies:
-
web-vitals (^2.1.4)
- Used in: client/src/reportWebVitals.js
- Called in: client/src/index.js line 20 as
reportWebVitals() - Purpose: React performance monitoring
- Recommendation: KEEP if you plan to monitor performance, REMOVE if not needed
-
@testing-library/ packages (^10.4.1, ^6.9.1, ^16.3.0)
- Used in: client/src/setupTests.js, client/src/App.test.js
- Purpose: React testing utilities
- Recommendation: KEEP if you plan to write tests, REMOVE if not running tests
-
@testing-library/user-event (^13.5.0)
- Not imported anywhere in src/
- Recommendation: REMOVE (unused testing utility)
-
skywatch (file:..)
- Self-reference to parent package
- Recommendation: REMOVE (circular dependency, not needed)
Action Items:
# Optional removals (if not using tests/vitals):
cd client
npm uninstall web-vitals @testing-library/user-event skywatch
# If removing web-vitals, also delete:
# - client/src/reportWebVitals.js
# - import/call in client/src/index.js lines 6, 20
# If removing testing libraries, also delete:
# - client/src/setupTests.js
# - client/src/App.test.js
4. Unused Files
Server
✅ All files are actively used:
- cache.js: Used in server/index.js
- database.js: Used in server/index.js, scheduler.js, monitorService.js, notificationService.js
- index.js: Main server entry point
- monitorService.js: Used in server/scheduler.js
- notificationService.js: Used in server/monitorService.js
- scheduler.js: Used in server/index.js
Client
⚠️ Potentially unused files:
-
client/src/reportWebVitals.js
- Purpose: Performance monitoring
- Used: Yes (called in index.js)
- Recommendation: Remove if not analyzing metrics
-
client/src/setupTests.js
- Purpose: Jest/testing configuration
- Used: Only if running tests
- Recommendation: Remove if not writing tests
-
client/src/App.test.js
- Purpose: Basic React component test
- Used: Only if running
npm test - Recommendation: Remove if not writing tests
5. Dead Code Patterns
Analysis: No major dead code found
Checked for:
- ✅ Unreachable functions
- ✅ Unused exports
- ✅ Orphaned modules
- ✅ Duplicate logic
Result: All implemented features are connected and functional
6. Optimization Opportunities
1. Consolidate console.log debugging
Current: Scattered debug logs throughout server/index.js Improvement: Add DEBUG environment variable flag
const DEBUG = process.env.DEBUG === 'true';
if (DEBUG) console.log('...');
2. Consider log levels
Current: Mix of console.log and console.error Improvement: Use logging library (winston, pino) for structured logging
npm install winston
3. Frontend error reporting
Current: Console errors only Improvement: Add error reporting service (Sentry, LogRocket) for production
Cleanup Priority
High Priority (Do Now):
- ✅ Remove debugging console.log statements from:
- server/index.js (timezone, history, SLA debug lines)
- server/notificationService.js (verbose email logs)
- client/src/History.js (API debug logs)
Medium Priority (Consider):
- Remove unused client dependencies if not using:
@testing-library/user-eventskywatch(circular dependency)- Testing files if not writing tests
- web-vitals if not monitoring performance
Low Priority (Future Improvement):
- Add structured logging with log levels
- Add DEBUG environment flag
- Consider production error monitoring
Implementation Commands
Immediate Cleanup (Non-Breaking):
# Remove debug console logs from server
# (Apply file edits as listed in Action Items section above)
# Remove unused client dependencies
cd client
npm uninstall @testing-library/user-event skywatch
Optional Cleanup (If not using tests/vitals):
cd client
npm uninstall web-vitals @testing-library/dom @testing-library/jest-dom @testing-library/react
rm src/reportWebVitals.js src/setupTests.js src/App.test.js
# Then remove imports from src/index.js
Validation Checklist
After cleanup, verify:
- Server starts without errors:
npm start - Client builds successfully:
cd client && npm run build - All features still work:
- CSV import/export
- Data retention settings
- WebSocket real-time updates
- Mobile responsive design
- Anomaly detection
- Alert rules (consecutive failures)
- Maintenance windows
- Resource tagging
- Transaction checks
- Trend graphs
Files Requiring Changes
High Priority Edits:
server/index.js- Remove 16 debug console.log linesserver/notificationService.js- Remove 3 verbose email logsclient/src/History.js- Remove 3 debug console.log linesclient/package.json- Remove 2 unused dependencies
Optional Edits (if removing tests/vitals):
client/src/index.js- Remove reportWebVitals import/call- Delete 3 client files: reportWebVitals.js, setupTests.js, App.test.js
client/package.json- Remove testing dependencies
Total Impact:
- Lines to remove: ~20 console statements
- Dependencies to remove: 2-6 packages
- Files to delete: 0-3 files (optional)
- Risk level: LOW (no functional changes, only cleanup)