|
1 | 1 | # XXX Comment Cleanup - TODO |
2 | 2 |
|
3 | | -**Status:** Partial - Phase 1-3 complete, 2 resolved + 1 reopened during audit |
| 3 | +**Status:** ✅ COMPLETE - All phases resolved |
4 | 4 | **Started:** 2025-11-25 |
5 | 5 | **Updated:** 2025-12-04 |
6 | 6 |
|
|
101 | 101 | - **Action:** Make CODE non-instantiable, update usages |
102 | 102 |
|
103 | 103 | ### 4.3 VPLS signature alignment |
104 | | -- **File:** `src/exabgp/bgp/message/update/nlri/vpls.py:71-107` |
105 | | -- **Status:** REOPENED - XXX removed but issue persists |
106 | | -- **Issue:** `make_vpls()` missing `action` and `addpath` params that other NLRIs have (INET, IPVPN, EVPN, Flow) |
107 | | -- **Action:** Add `action: Action = Action.ANNOUNCE` and `addpath: PathInfo = PathInfo.DISABLED` to `make_vpls()` |
| 104 | +- **File:** `src/exabgp/bgp/message/update/nlri/vpls.py:73-105` |
| 105 | +- **Status:** ✅ RESOLVED - `make_vpls()` has `action` and `addpath` params |
| 106 | +- **Also:** `make_empty()` factory method added with same params (lines 107-125) |
108 | 107 |
|
109 | 108 | ### 4.4 TrafficNextHopSimpson inheritance |
110 | | -- **File:** `src/exabgp/bgp/message/update/attribute/community/extended/traffic.py:277` |
111 | | -- **Status:** Pending |
112 | | -- **Action:** Make subclass of NextHop or IP |
| 109 | +- **File:** `src/exabgp/bgp/message/update/attribute/community/extended/traffic.py:282-304` |
| 110 | +- **Status:** ✅ RESOLVED - No change needed |
| 111 | +- **Analysis:** Current design is correct. TrafficNextHopSimpson is an ExtendedCommunity (not NextHop/IP) because it doesn't contain an IP address - it only signals "use the UPDATE's existing NextHop" with a copy flag. Inheriting from NextHop or IP would be semantically wrong. |
113 | 112 |
|
114 | 113 | --- |
115 | 114 |
|
116 | | -## Phase 5: Investigation Required - NOT STARTED |
| 115 | +## Phase 5: Investigation Required - ✅ COMPLETE |
117 | 116 |
|
118 | 117 | ### 5.1 VPLS unique key |
119 | | -- **File:** `src/exabgp/bgp/message/update/nlri/vpls.py:218-219` |
120 | | -- **Status:** Pending investigation |
| 118 | +- **File:** `src/exabgp/bgp/message/update/nlri/vpls.py:237-240` |
| 119 | +- **Status:** ✅ RESOLVED - XXX removed, unique key = all fields (rd, endpoint, base, offset, size) documented |
121 | 120 |
|
122 | 121 | ### 5.2 RTC variable length prefixing |
123 | | -- **File:** `src/exabgp/bgp/message/update/nlri/rtc.py:37` |
124 | | -- **Status:** Pending investigation |
| 122 | +- **File:** `src/exabgp/bgp/message/update/nlri/rtc.py:30-38` |
| 123 | +- **Status:** ✅ RESOLVED - XXX removed, limitation documented in docstring (RFC 4684 prefix filtering not implemented) |
125 | 124 |
|
126 | 125 | ### 5.3 EVPN MAC index |
127 | | -- **File:** `src/exabgp/bgp/message/update/nlri/evpn/mac.py:132` |
128 | | -- **Status:** Pending investigation |
| 126 | +- **File:** `src/exabgp/bgp/message/update/nlri/evpn/mac.py:132-138` |
| 127 | +- **Status:** ✅ RESOLVED - XXX removed, design documented: index() uses full bytes, __eq__() uses RFC key fields |
129 | 128 |
|
130 | 129 | ### 5.4 SRCAP redundant parsing |
131 | | -- **File:** `src/exabgp/bgp/message/update/attribute/bgpls/node/srcap.py:92` |
132 | | -- **Status:** Pending investigation |
| 130 | +- **File:** `src/exabgp/bgp/message/update/attribute/bgpls/node/srcap.py:88-91` |
| 131 | +- **Status:** ✅ RESOLVED - XXX removed, offset calculation (7 = 3 range + 4 header) is correct, not redundant |
133 | 132 |
|
134 | 133 | ### 5.5 PMSI length discrepancy |
135 | 134 | - **File:** `src/exabgp/bgp/message/update/attribute/pmsi.py:90` |
136 | 135 | - **Status:** ✅ RESOLVED - XXX comment no longer exists |
137 | 136 |
|
138 | 137 | ### 5.6 BGP-LS IGP tags LEN checks |
139 | 138 | - **Files:** `igpextags.py:31`, `igptags.py:34` |
140 | | -- **Status:** Pending investigation |
| 139 | +- **Status:** ✅ RESOLVED - XXX removed, cls.check(data) validates length, comments document expected format |
141 | 140 |
|
142 | 141 | --- |
143 | 142 |
|
|
152 | 151 | | 2025-11-25 | Phase 3 implementation | Caching init removed, comments clarified | |
153 | 152 | | 2025-11-25 | All tests pass | Ready for commit | |
154 | 153 | | 2025-12-04 | Validation audit | 2 resolved (4.1, 5.5), 4.3 reopened (make_vpls needs action/addpath), line numbers updated | |
| 154 | +| 2025-12-04 | Recheck 4.3 | 4.3 resolved - make_vpls() has action/addpath params | |
| 155 | +| 2025-12-04 | Investigate 4.4 | 4.4 resolved - current design correct, no inheritance change needed | |
| 156 | +| 2025-12-04 | Investigate Phase 5 | All 6 items resolved - XXX comments replaced with documentation in commit c948819c | |
155 | 157 |
|
156 | 158 | --- |
157 | 159 |
|
|
0 commit comments