Skip to content

refactor: add Gap to bull_ui and drop gap dependency - #2558

Merged
ethicnology merged 3 commits into
developfrom
refactor/gap-through-bull-ui
Aug 4, 2026
Merged

refactor: add Gap to bull_ui and drop gap dependency#2558
ethicnology merged 3 commits into
developfrom
refactor/gap-through-bull-ui

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

Removes the external gap package from the project.

bull_ui now ships its own Gap (vendored from gap 3.0.1), and every feature sources Gap through bull_ui instead of importing package:gap.

@wired-pasteque wired-pasteque self-assigned this Aug 4, 2026
@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 3m 6s —— View job


PR Review — vendoring Gap into bull_ui

  • Gather context (diff vs origin/develop)
  • Review vendored Gap implementation against gap 3.0.1
  • Check import rewiring for leftovers/regressions
  • Verify pubspec/dependency cleanup
  • Post review findings

Summary: no correctness bugs, security issues, or regressions found. The vendored implementation is behaviorally identical to upstream gap 3.0.1, and the 250+ import swaps are clean and mechanical. Two minor, non-blocking suggestions below.

(Note: I can't submit formal inline review comments, so findings are consolidated here with file/line references.)

What I verified

  • Vendored code fidelity (packages/bull_ui/lib/src/layout/gap.dart): compared Gap, _RawGap, and RenderGap against upstream 3.0.1 — constructors and asserts, the Scrollable.maybeOf fallback (the fix that 3.0.1 introduced), the RenderFlex direction lookup, intrinsic sizing, computeDryLayout/performLayout, the color paint path, and the markNeedsLayout/markNeedsPaint setters all match. The only changes are cosmetic (super-parameters, doc comments), as the header claims.
  • No leftovers: zero remaining package:gap references in code, tests, or any pubspec; gap is removed from root pubspec.yaml, packages/bull_ui/pubspec.yaml, and pubspec.lock consistently. MaxGap/SliverGap are confirmed unused in the repo, so dropping them loses nothing.
  • Import swaps: app code uses import 'package:bull_ui/bull_ui.dart' show Gap;, which is a good call — it avoids pulling bull_ui's broad Material re-exports into scope of ~250 files. bull_ui internals correctly use the direct src/layout/gap.dart import. I checked for duplicate bull_ui imports introduced by the swap and found none. bull_ui.dart exports only Gap (not RenderGap), which nothing else needs.

Minor suggestions (non-blocking)

  1. License text (packages/bull_ui/lib/src/layout/gap.dart:3): the header credits the author and names the MIT license, but MIT's condition is that the full license text accompany substantial copies. Consider pasting the full MIT text into the header or adding a third-party-notices entry. Fix this →
  2. Test coverage: packages/bull_ui/test/ has widget tests for the other components but none for the vendored Gap. Now that the code is owned in-repo, porting upstream's small test suite (sizing in Row/Column, the Scrollable fallback, the error when no direction is resolvable) would guard the vendored copy against future edits. Fix this →

One caveat: this runner has no Flutter SDK, so I couldn't run flutter analyze/flutter test — the existing CI checks should confirm compilation.

@ethicnology
ethicnology merged commit d6f5d4f into develop Aug 4, 2026
2 checks passed
ethicnology added a commit that referenced this pull request Aug 4, 2026
Resolutions worth knowing about:

- gap -> bull_ui: develop vendored Gap into bull_ui and dropped the `gap` dependency (#2558). The three tor/recoverbull widgets that still imported `package:gap/gap.dart` now import `Gap` from bull_ui. Note that pubspec.yaml auto-merged without conflict, so the dependency was already gone while those imports remained — the compiler, not git, caught it.
- connecting_page: took bull_ui's Gap and dropped the `gif` import, which this branch had already removed along with the animation.
- recoverbull_repository: develop swapped `package:hex` for `package:convert` (`convert.hex.decode(_normalizeHex(x))`) while this branch changed `externalProxy:` to `endpoint:`. Orthogonal, so both sides kept.
- recoverbull_repository_test: git auto-merged this file with no conflict, but develop's new hex-tolerance group still called the pre-branch signature. Ported to the 4-positional-argument `fetchVaultKey` and `endpoint:` stub.
- localization: took develop's 27 .arb wholesale, then replayed this branch's 21 Tor keys and its updated torSettingsInfoDescription through tools/arb.dart, so every other key stays byte-for-byte unchanged. Verified per locale: develop's keys untouched, the branch's per-locale translations restored.

Ran build-runner (TorSettingsState and SettingsEntity both gained fields), then `make checks` green: analyze, bull-ui-check, fix-check, format-check, and 940 + 21 + 1 + 23 unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants