Replies: 1 comment
|
I don’t lead this project, so I may not be the right person to direct this proposal to. That said, I do think the information is useful and worth considering. From my perspective, the extended review process is primarily driven by three factors:
Ultimately, I believe the extended review cycle comes down to three closely related factors:
One practical way to reduce this is to challenge each proposed fix before accepting it. Ask whether the change stays within the PR’s intended scope, whether it introduces unnecessary refactoring, and whether the same issue exists elsewhere in the relevant code. A rapidly growing PR is often a useful warning sign that scope drift may be occurring. Stepping back and reviewing how the PR expanded can help identify where the model chose an overly broad or expedient solution. It can also reveal cases where the reported issue already exists on the main branch and would be better handled in a separate PR. |
Uh oh!
There was an error while loading. Please reload this page.
Hi everyone, cc @jatmn,
First of all, thank you for the thorough and rigorous reviews on recent PRs (such as #953). The high bar on concurrency invariants and platform safety is truly appreciated.
Going through these successive review rounds got me reflecting on our debugging ergonomics: I rarely experienced this much ping-pong while tracking down lifecycle and UI regressions, and it made me wonder whether our current repository structure and state coupling might be contributing to these cascading side effects.
When working inside
internal/tui/model.go(and acrossinternal/in general), I noticed how easily a change targeting a specific view or an async loading path can inadvertently cross paths with other components (modal routing, git sweeps, or terminal resizing) due to shared state fields and distributed lifecycle checks.To explore this, I put together an experimental spike on a separate branch in my fork (
proposal/decoupled-architecture) with a structure that feels more modular and familiar to me, trying to see if we could naturally minimize blast radius and isolate unit tests.I documented some thoughts in
docs/repostructure.md, but I would love to get your perspective and humble advice on a few design questions:1. Pure, Zero-Dependency Utility Packages (
pkg/)Would there be value in extracting completely self-contained, pure algorithms out of
internal/tui/into standalonepkg/packages?pkg/difftool: purely handles canonical line key normalizations (CanonicalLineKey,MatchChangedLines) so that terminal presentation transforms (tabs, control bytes) can never interfere with diff gutter alignment (▎).pkg/ansi: dedicated to terminal string sanitization (SanitizeFileLine).pkg/safefs: houses non-wrapping integer bounds arithmetic (InBoundsNonWrappingforpkg/packages (which execute unit tests in ~2ms with zero UI setup) align with the long-term vision of Zero, or is keeping all helpers underinternal/an intentional boundary design?2. Unified Monotonic Authority Store vs. Distributed State Fields
In #953, moving to a monotonically increasing source revision (
pathRevisions map[string]uint64) proved to be the key to eliminating race conditions where intermediate resize/theme events could drop required source refreshes.internal/ui/state.MonotonicAuthority(liveSeq,lifetimeToken,pathRevisions,generation).modelstruct?3. Structuring
internal/tui/by Sub-domainsWith
internal/tui/currently holding ~250 files and over 90k lines in a flat directory, navigating and assessing caller impact can be challenging.ui/views,ui/components,ui/layout,ui/state), or does the current flatinternal/tuilayout provide specific advantages for Bubbletea model composition that I might be overlooking?Everything on the experimental branch is fully green under
go test -race -count=1 ./...across all 83 packages, but this is purely shared as food for thought and an open conversation starter.I would love to hear your thoughts on whether any of these patterns could be helpful as Zero continues to scale!
Thanks again for your time and feedback.
All reactions