Add trends UI, cleanup logs, refresh build
This commit is contained in:
nmemmert committed
1 parent
71d1dbd5c1
commit
d4e4467c0c
20 files changed
+1739
-90
No files matched your search
@@ -0,0 +1,283 @@
|
||||
# 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)
|
||||
Reference in new issue
Block a user