284 lines
8.6 KiB
Markdown
284 lines
8.6 KiB
Markdown
# 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:**
|
|
|
|
1. **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
|
|
|
|
2. **@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
|
|
|
|
3. **@testing-library/user-event** (^13.5.0)
|
|
- Not imported anywhere in src/
|
|
- **Recommendation:** REMOVE (unused testing utility)
|
|
|
|
4. **skywatch** (file:..)
|
|
- Self-reference to parent package
|
|
- **Recommendation:** REMOVE (circular dependency, not needed)
|
|
|
|
**Action Items:**
|
|
```bash
|
|
# 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:**
|
|
|
|
1. **client/src/reportWebVitals.js**
|
|
- Purpose: Performance monitoring
|
|
- Used: Yes (called in index.js)
|
|
- **Recommendation:** Remove if not analyzing metrics
|
|
|
|
2. **client/src/setupTests.js**
|
|
- Purpose: Jest/testing configuration
|
|
- Used: Only if running tests
|
|
- **Recommendation:** Remove if not writing tests
|
|
|
|
3. **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
|
|
```javascript
|
|
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
|
|
```bash
|
|
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):
|
|
1. ✅ 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):
|
|
2. Remove unused client dependencies if not using:
|
|
- `@testing-library/user-event`
|
|
- `skywatch` (circular dependency)
|
|
- Testing files if not writing tests
|
|
- web-vitals if not monitoring performance
|
|
|
|
### Low Priority (Future Improvement):
|
|
3. Add structured logging with log levels
|
|
4. Add DEBUG environment flag
|
|
5. Consider production error monitoring
|
|
|
|
---
|
|
|
|
## Implementation Commands
|
|
|
|
### Immediate Cleanup (Non-Breaking):
|
|
```bash
|
|
# 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):
|
|
```bash
|
|
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:
|
|
1. `server/index.js` - Remove 16 debug console.log lines
|
|
2. `server/notificationService.js` - Remove 3 verbose email logs
|
|
3. `client/src/History.js` - Remove 3 debug console.log lines
|
|
4. `client/package.json` - Remove 2 unused dependencies
|
|
|
|
### Optional Edits (if removing tests/vitals):
|
|
5. `client/src/index.js` - Remove reportWebVitals import/call
|
|
6. Delete 3 client files: reportWebVitals.js, setupTests.js, App.test.js
|
|
7. `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)
|