Browse Source
- Transform obsolete historical comments into actionable architectural guidance - Remove references to removed turnOffNotifyingFlags method - Update notification settings cleanup comments to reflect current architecture - Create new Harbor Pilot rule for historical comment management - Integrate historical comment management into main Harbor Pilot directive - Improve code clarity for future developers by documenting architectural evolution Addresses team feedback about historical comment value and maintains important migration context while removing clutter.pull/150/head
3 changed files with 241 additions and 4 deletions
@ -0,0 +1,236 @@ |
|||||
|
--- |
||||
|
description: when comments are generated by the model |
||||
|
alwaysApply: false |
||||
|
--- |
||||
|
# Historical Comment Management — Harbor Pilot Directive |
||||
|
|
||||
|
> **Agent role**: When encountering historical comments about removed methods, deprecated patterns, or architectural changes, apply these guidelines to maintain code clarity and developer guidance. |
||||
|
|
||||
|
## 🎯 Purpose |
||||
|
|
||||
|
Historical comments should either be **removed entirely** or **transformed into actionable guidance** for future developers. Avoid keeping comments that merely state what was removed without explaining why or what to do instead. |
||||
|
|
||||
|
## 📋 Decision Framework |
||||
|
|
||||
|
### Remove Historical Comments When: |
||||
|
- **Obsolete Information**: Comment describes functionality that no longer exists |
||||
|
- **No Action Required**: Comment doesn't help future developers make decisions |
||||
|
- **Outdated Context**: Comment refers to old patterns that are no longer relevant |
||||
|
- **Self-Evident**: The current code clearly shows the current approach |
||||
|
|
||||
|
### Transform Historical Comments When: |
||||
|
- **Architectural Context**: The change represents a significant pattern shift |
||||
|
- **Migration Guidance**: Future developers might need to understand the evolution |
||||
|
- **Decision Rationale**: The "why" behind the change is still relevant |
||||
|
- **Alternative Approaches**: The comment can guide future implementation choices |
||||
|
|
||||
|
## 🔄 Transformation Patterns |
||||
|
|
||||
|
### 1. From Removal Notice to Migration Note |
||||
|
```typescript |
||||
|
// ❌ REMOVE THIS |
||||
|
// turnOffNotifyingFlags method removed - notification state is now managed by NotificationSection component |
||||
|
|
||||
|
// ✅ TRANSFORM TO THIS |
||||
|
// Note: Notification state management has been migrated to NotificationSection component |
||||
|
// which handles its own lifecycle and persistence via PlatformServiceMixin |
||||
|
``` |
||||
|
|
||||
|
### 2. From Deprecation Notice to Implementation Guide |
||||
|
```typescript |
||||
|
// ❌ REMOVE THIS |
||||
|
// This will be handled by the NewComponent now |
||||
|
// No need to call oldMethod() as it's no longer needed |
||||
|
|
||||
|
// ✅ TRANSFORM TO THIS |
||||
|
// Note: This functionality has been migrated to NewComponent |
||||
|
// which provides better separation of concerns and testability |
||||
|
``` |
||||
|
|
||||
|
### 3. From Historical Note to Architectural Context |
||||
|
```typescript |
||||
|
// ❌ REMOVE THIS |
||||
|
// Old approach: used direct database calls |
||||
|
// New approach: uses service layer |
||||
|
|
||||
|
// ✅ TRANSFORM TO THIS |
||||
|
// Note: Database access has been abstracted through service layer |
||||
|
// for better testability and platform independence |
||||
|
``` |
||||
|
|
||||
|
## 🚫 Anti-Patterns to Remove |
||||
|
|
||||
|
- Comments that only state what was removed |
||||
|
- Comments that don't explain the current approach |
||||
|
- Comments that reference non-existent methods |
||||
|
- Comments that are self-evident from the code |
||||
|
- Comments that don't help future decision-making |
||||
|
|
||||
|
## ✅ Best Practices |
||||
|
|
||||
|
### When Keeping Historical Context: |
||||
|
1. **Explain the "Why"**: Why was the change made? |
||||
|
2. **Describe the "What"**: What is the current approach? |
||||
|
3. **Provide Context**: When might this information be useful? |
||||
|
4. **Use Actionable Language**: Guide future decisions, not just document history |
||||
|
|
||||
|
### When Removing Historical Context: |
||||
|
1. **Verify Obsoleteness**: Ensure the information is truly outdated |
||||
|
2. **Check for Dependencies**: Ensure no other code references the old approach |
||||
|
3. **Update Related Docs**: If removing from code, consider adding to documentation |
||||
|
4. **Preserve in Git History**: The change is preserved in version control |
||||
|
|
||||
|
## 🔍 Implementation Checklist |
||||
|
|
||||
|
- [ ] Identify historical comments about removed/deprecated functionality |
||||
|
- [ ] Determine if comment provides actionable guidance |
||||
|
- [ ] Transform useful comments into migration notes or architectural context |
||||
|
- [ ] Remove comments that are purely historical without guidance value |
||||
|
- [ ] Ensure remaining comments explain current approach and rationale |
||||
|
- [ ] Update related documentation if significant context is removed |
||||
|
|
||||
|
## 📚 Examples |
||||
|
|
||||
|
### Good Historical Comment (Keep & Transform) |
||||
|
```typescript |
||||
|
// Note: Database access has been migrated from direct IndexedDB calls to PlatformServiceMixin |
||||
|
// This provides better platform abstraction and consistent error handling across web/mobile/desktop |
||||
|
// When adding new database operations, use this.$getContact(), this.$saveSettings(), etc. |
||||
|
``` |
||||
|
|
||||
|
### Bad Historical Comment (Remove) |
||||
|
```typescript |
||||
|
// Old method getContactFromDB() removed - now handled by PlatformServiceMixin |
||||
|
// No need to call the old method anymore |
||||
|
``` |
||||
|
|
||||
|
## 🎯 Integration with Harbor Pilot |
||||
|
|
||||
|
This rule works in conjunction with: |
||||
|
- **Component Creation Ideals**: Maintains architectural consistency |
||||
|
- **Migration Patterns**: Documents evolution of patterns |
||||
|
- **Code Review Guidelines**: Ensures comments provide value |
||||
|
|
||||
|
## 📝 Version History |
||||
|
|
||||
|
### v1.0.0 (2025-08-21) |
||||
|
- Initial creation based on notification system cleanup |
||||
|
- Established decision framework for historical comment management |
||||
|
- Added transformation patterns and anti-patterns |
||||
|
- Integrated with existing Harbor Pilot architecture rules |
||||
|
# Historical Comment Management — Harbor Pilot Directive |
||||
|
|
||||
|
> **Agent role**: When encountering historical comments about removed methods, deprecated patterns, or architectural changes, apply these guidelines to maintain code clarity and developer guidance. |
||||
|
|
||||
|
## 🎯 Purpose |
||||
|
|
||||
|
Historical comments should either be **removed entirely** or **transformed into actionable guidance** for future developers. Avoid keeping comments that merely state what was removed without explaining why or what to do instead. |
||||
|
|
||||
|
## 📋 Decision Framework |
||||
|
|
||||
|
### Remove Historical Comments When: |
||||
|
- **Obsolete Information**: Comment describes functionality that no longer exists |
||||
|
- **No Action Required**: Comment doesn't help future developers make decisions |
||||
|
- **Outdated Context**: Comment refers to old patterns that are no longer relevant |
||||
|
- **Self-Evident**: The current code clearly shows the current approach |
||||
|
|
||||
|
### Transform Historical Comments When: |
||||
|
- **Architectural Context**: The change represents a significant pattern shift |
||||
|
- **Migration Guidance**: Future developers might need to understand the evolution |
||||
|
- **Decision Rationale**: The "why" behind the change is still relevant |
||||
|
- **Alternative Approaches**: The comment can guide future implementation choices |
||||
|
|
||||
|
## 🔄 Transformation Patterns |
||||
|
|
||||
|
### 1. From Removal Notice to Migration Note |
||||
|
```typescript |
||||
|
// ❌ REMOVE THIS |
||||
|
// turnOffNotifyingFlags method removed - notification state is now managed by NotificationSection component |
||||
|
|
||||
|
// ✅ TRANSFORM TO THIS |
||||
|
// Note: Notification state management has been migrated to NotificationSection component |
||||
|
// which handles its own lifecycle and persistence via PlatformServiceMixin |
||||
|
``` |
||||
|
|
||||
|
### 2. From Deprecation Notice to Implementation Guide |
||||
|
```typescript |
||||
|
// ❌ REMOVE THIS |
||||
|
// This will be handled by the NewComponent now |
||||
|
// No need to call oldMethod() as it's no longer needed |
||||
|
|
||||
|
// ✅ TRANSFORM TO THIS |
||||
|
// Note: This functionality has been migrated to NewComponent |
||||
|
// which provides better separation of concerns and testability |
||||
|
``` |
||||
|
|
||||
|
### 3. From Historical Note to Architectural Context |
||||
|
```typescript |
||||
|
// ❌ REMOVE THIS |
||||
|
// Old approach: used direct database calls |
||||
|
// New approach: uses service layer |
||||
|
|
||||
|
// ✅ TRANSFORM TO THIS |
||||
|
// Note: Database access has been abstracted through service layer |
||||
|
// for better testability and platform independence |
||||
|
``` |
||||
|
|
||||
|
## 🚫 Anti-Patterns to Remove |
||||
|
|
||||
|
- Comments that only state what was removed |
||||
|
- Comments that don't explain the current approach |
||||
|
- Comments that reference non-existent methods |
||||
|
- Comments that are self-evident from the code |
||||
|
- Comments that don't help future decision-making |
||||
|
|
||||
|
## ✅ Best Practices |
||||
|
|
||||
|
### When Keeping Historical Context: |
||||
|
1. **Explain the "Why"**: Why was the change made? |
||||
|
2. **Describe the "What"**: What is the current approach? |
||||
|
3. **Provide Context**: When might this information be useful? |
||||
|
4. **Use Actionable Language**: Guide future decisions, not just document history |
||||
|
|
||||
|
### When Removing Historical Context: |
||||
|
1. **Verify Obsoleteness**: Ensure the information is truly outdated |
||||
|
2. **Check for Dependencies**: Ensure no other code references the old approach |
||||
|
3. **Update Related Docs**: If removing from code, consider adding to documentation |
||||
|
4. **Preserve in Git History**: The change is preserved in version control |
||||
|
|
||||
|
## 🔍 Implementation Checklist |
||||
|
|
||||
|
- [ ] Identify historical comments about removed/deprecated functionality |
||||
|
- [ ] Determine if comment provides actionable guidance |
||||
|
- [ ] Transform useful comments into migration notes or architectural context |
||||
|
- [ ] Remove comments that are purely historical without guidance value |
||||
|
- [ ] Ensure remaining comments explain current approach and rationale |
||||
|
- [ ] Update related documentation if significant context is removed |
||||
|
|
||||
|
## 📚 Examples |
||||
|
|
||||
|
### Good Historical Comment (Keep & Transform) |
||||
|
```typescript |
||||
|
// Note: Database access has been migrated from direct IndexedDB calls to PlatformServiceMixin |
||||
|
// This provides better platform abstraction and consistent error handling across web/mobile/desktop |
||||
|
// When adding new database operations, use this.$getContact(), this.$saveSettings(), etc. |
||||
|
``` |
||||
|
|
||||
|
### Bad Historical Comment (Remove) |
||||
|
```typescript |
||||
|
// Old method getContactFromDB() removed - now handled by PlatformServiceMixin |
||||
|
// No need to call the old method anymore |
||||
|
``` |
||||
|
|
||||
|
## 🎯 Integration with Harbor Pilot |
||||
|
|
||||
|
This rule works in conjunction with: |
||||
|
- **Component Creation Ideals**: Maintains architectural consistency |
||||
|
- **Migration Patterns**: Documents evolution of patterns |
||||
|
- **Code Review Guidelines**: Ensures comments provide value |
||||
|
|
||||
|
## 📝 Version History |
||||
|
|
||||
|
### v1.0.0 (2025-08-21) |
||||
|
- Initial creation based on notification system cleanup |
||||
|
- Established decision framework for historical comment management |
||||
|
- Added transformation patterns and anti-patterns |
||||
|
- Integrated with existing Harbor Pilot architecture rules |
Loading…
Reference in new issue