|
24 | 24 | - Rebased on origin/main successfully |
25 | 25 | - Implemented integration (opts, periodic measurement, metrics) |
26 | 26 | - Ran internal code review (review-round-1.md) |
27 | | -- Addressed review findings: |
| 27 | +- Addressed internal review findings: |
28 | 28 | - Removed `String.to_atom/1` from `table_type/1` to prevent atom exhaustion |
29 | 29 | - Handled race condition when table deleted between `:ets.all()` and `:ets.info/2` |
30 | 30 | - Fixed test assertions: replaced silent `if` guards with proper `assert` calls |
31 | 31 | - Added `on_exit` callbacks for reliable ETS table cleanup in tests |
32 | 32 | - Fixed test table names to use UUID-like suffixes for correct type grouping |
33 | 33 | - All 13 tests passing, compilation clean with --warnings-as-errors |
34 | 34 | - Force-pushed rebased branch, updated PR description, added `claude` label |
| 35 | +- CI: electric-telemetry build passes, formatting passes |
| 36 | +- CI: `CallHomeReporterTest` has pre-existing flaky failures (unrelated to our changes) |
| 37 | +- Claude Code Review bot (round 1): passed, feedback addressed |
| 38 | + - Fixed docstring examples to show string types instead of atoms |
| 39 | + - Hoisted `:erlang.system_info(:wordsize)` to module attribute |
| 40 | + - Dismissed `.agent-tasks` feedback (intentional per project convention) |
| 41 | +- Claude Code Review bot (round 2): timed out (infrastructure issue), no new feedback posted |
35 | 42 |
|
36 | 43 | ### Dismissed review items |
37 | 44 | - UUID regex too greedy for underscores: Edge case for Electric's specific naming patterns, acceptable |
38 | 45 | - Inconsistent field naming (type_table_count vs table_count): Different data structures with different semantics |
39 | 46 | - `top_memory_stats` unused in integration: Intended for IEx debugging |
40 | 47 | - Performance concern about polling: Same trade-off as existing process_memory, acceptable |
| 48 | +- Two sequential reduce passes: Not worth the complexity for a minor optimization |
| 49 | +- Dead code branch (`if count > 0`): Harmless defensive guard |
41 | 50 |
|
42 | 51 | ### Operational issues |
43 | 52 | - Old worktree existed at `/home/alco/code/workspaces/electric/ets-observability`, had to remove it first |
44 | 53 | - Husky pre-commit hook not executable (warning only, doesn't block) |
| 54 | +- Named ETS tables in tests auto-deleted when test process exits; `on_exit` callbacks need try/rescue not `:ets.info` checks |
| 55 | +- `mix format` required for try/rescue blocks in `on_exit` (one-liners not allowed) |
0 commit comments