sneak/pixa
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

refactor: extract whitelist package from internal/imgcache #41

Merged
sneak merged 2 commits from refactor/extract-whitelist-package into main 2026-03-25 20:44:57 +01:00
Conversation 4 Commits 2 Files Changed 3 +31 -27
clawbot commented 2026-03-25 02:23:01 +01:00
Collaborator
Copy Link

Extract HostWhitelist, NewHostWhitelist, IsWhitelisted, IsEmpty, and Count from internal/imgcache/ into the new internal/whitelist/ package.

The whitelist package is completely self-contained, depending only on net/url and strings from the standard library. No circular imports introduced.

Changes:

  • Moved whitelist.go → internal/whitelist/whitelist.go (added package comment)
  • Moved whitelist_test.go → internal/whitelist/whitelist_test.go (adapted to external test style)
  • Updated internal/imgcache/service.go to import from sneak.berlin/go/pixa/internal/whitelist

docker build . passes (lint, tests, build).

Part of issue #39

Extract `HostWhitelist`, `NewHostWhitelist`, `IsWhitelisted`, `IsEmpty`, and `Count` from `internal/imgcache/` into the new `internal/whitelist/` package. The whitelist package is completely self-contained, depending only on `net/url` and `strings` from the standard library. No circular imports introduced. **Changes:** - Moved `whitelist.go` → `internal/whitelist/whitelist.go` (added package comment) - Moved `whitelist_test.go` → `internal/whitelist/whitelist_test.go` (adapted to external test style) - Updated `internal/imgcache/service.go` to import from `sneak.berlin/go/pixa/internal/whitelist` `docker build .` passes (lint, tests, build). Part of [issue #39](https://git.eeqj.de/sneak/pixa/issues/39)
clawbot added 1 commit 2026-03-25 02:23:01 +01:00
refactor: extract whitelist package from internal/imgcache
All checks were successful
check / check (push) Successful in 57s
Details
eae6f6e9ac 
Move HostWhitelist, NewHostWhitelist, IsWhitelisted, IsEmpty, and Count
from internal/imgcache into the new internal/whitelist package.

The whitelist package is completely self-contained, depending only on
net/url and strings from the standard library.

Updated import in internal/imgcache/service.go to use the new package.
Tests moved and adapted to external test style (package whitelist_test).

Part of #39
clawbot added the needs-review label 2026-03-25 02:23:35 +01:00
clawbot commented 2026-03-25 02:25:52 +01:00
Author
Collaborator
Copy Link

Review: PR #41 — extract whitelist package

Policy Compliance

Check Result
No linter/CI/test config changes ✅ No changes to .golangci.yml, Makefile, Dockerfile, .gitea/workflows/
No logic changes in extracted code ✅ whitelist.go body identical to main — only package declaration and doc comment changed
Tests preserved with full assertions ✅ All test cases, assertions, and expected values identical. Test style changed from internal (package imgcache) to external (package whitelist_test) — correct for a standalone package
Import sites updated ✅ service.go imports sneak.berlin/go/pixa/internal/whitelist and uses whitelist.HostWhitelist / whitelist.NewHostWhitelist
No circular imports ✅ internal/whitelist/ depends only on net/url and strings — zero internal deps
No scope creep ✅ Exactly 3 files changed: move whitelist.go, move whitelist_test.go, update service.go imports
Old files removed ✅ internal/imgcache/whitelist.go and whitelist_test.go no longer exist
Package doc comment ✅ Added // Package whitelist provides host-based URL whitelisting for the image proxy.

Requirement Verification

  1. ✅ whitelist.go moved cleanly to internal/whitelist/ with no logic changes
  2. ✅ whitelist_test.go moved with package whitelist_test (external test style)
  3. ✅ All import sites updated (service.go)
  4. ✅ No circular imports
  5. ✅ No other packages extracted (only whitelist)
  6. ✅ No linter/CI/test config changes
  7. ✅ docker build . passes — all tests pass, binary builds successfully

Docker Build

ok  sneak.berlin/go/pixa/internal/whitelist  0.003s
ok  sneak.berlin/go/pixa/internal/imgcache   0.166s
... all packages PASS
Image built successfully.

Verdict: PASS ✅

Clean, minimal extraction. No logic changes, no test weakening, no config modifications. Correctly scoped to issue #39.

## Review: PR #41 — extract whitelist package ### Policy Compliance | Check | Result | |---|---| | No linter/CI/test config changes | ✅ No changes to `.golangci.yml`, `Makefile`, `Dockerfile`, `.gitea/workflows/` | | No logic changes in extracted code | ✅ `whitelist.go` body identical to `main` — only `package` declaration and doc comment changed | | Tests preserved with full assertions | ✅ All test cases, assertions, and expected values identical. Test style changed from internal (`package imgcache`) to external (`package whitelist_test`) — correct for a standalone package | | Import sites updated | ✅ `service.go` imports `sneak.berlin/go/pixa/internal/whitelist` and uses `whitelist.HostWhitelist` / `whitelist.NewHostWhitelist` | | No circular imports | ✅ `internal/whitelist/` depends only on `net/url` and `strings` — zero internal deps | | No scope creep | ✅ Exactly 3 files changed: move whitelist.go, move whitelist_test.go, update service.go imports | | Old files removed | ✅ `internal/imgcache/whitelist.go` and `whitelist_test.go` no longer exist | | Package doc comment | ✅ Added `// Package whitelist provides host-based URL whitelisting for the image proxy.` | ### Requirement Verification 1. ✅ `whitelist.go` moved cleanly to `internal/whitelist/` with no logic changes 2. ✅ `whitelist_test.go` moved with `package whitelist_test` (external test style) 3. ✅ All import sites updated (`service.go`) 4. ✅ No circular imports 5. ✅ No other packages extracted (only whitelist) 6. ✅ No linter/CI/test config changes 7. ✅ `docker build .` passes — all tests pass, binary builds successfully ### Docker Build ``` ok sneak.berlin/go/pixa/internal/whitelist 0.003s ok sneak.berlin/go/pixa/internal/imgcache 0.166s ... all packages PASS Image built successfully. ``` ### Verdict: **PASS** ✅ Clean, minimal extraction. No logic changes, no test weakening, no config modifications. Correctly scoped to [issue #39](https://git.eeqj.de/sneak/pixa/issues/39).
clawbot added merge-ready and removed needs-review labels 2026-03-25 02:28:20 +01:00
sneak was assigned by clawbot 2026-03-25 02:28:21 +01:00
sneak commented 2026-03-25 02:31:05 +01:00
Owner
Copy Link

These stutter. Constructors should be named .New() whenever possible. Also it’s a HostAllowList, not a generic “whitelist”.

These stutter. Constructors should be named .New() whenever possible. Also it’s a HostAllowList, not a generic “whitelist”.
clawbot added needs-rework and removed merge-ready labels 2026-03-25 02:32:19 +01:00
sneak was unassigned by clawbot 2026-03-25 02:32:20 +01:00
clawbot self-assigned this 2026-03-25 02:32:20 +01:00
clawbot force-pushed refactor/extract-whitelist-package from eae6f6e9ac to 469b8bf547 2026-03-25 02:36:05 +01:00 Compare
clawbot commented 2026-03-25 02:36:12 +01:00
Author
Collaborator
Copy Link

Rework: rename to allowlist, fix stuttering

Addressed the review feedback:

  1. Package renamed internal/whitelist/ → internal/allowlist/
  2. Type renamed HostWhitelist → HostAllowList
  3. Constructor renamed NewHostWhitelist() → New() — usage is now allowlist.New(...) (no stutter)
  4. Method renamed IsWhitelisted() → IsAllowed()
  5. All references updated in service.go (import, field, constructor call, method call, comments)
  6. No logic changes — purely rename/reorganization

docker build . passes (fmt, lint, all tests, binary build).

## Rework: rename to allowlist, fix stuttering Addressed the review feedback: 1. **Package renamed** `internal/whitelist/` → `internal/allowlist/` 2. **Type renamed** `HostWhitelist` → `HostAllowList` 3. **Constructor renamed** `NewHostWhitelist()` → `New()` — usage is now `allowlist.New(...)` (no stutter) 4. **Method renamed** `IsWhitelisted()` → `IsAllowed()` 5. **All references updated** in `service.go` (import, field, constructor call, method call, comments) 6. **No logic changes** — purely rename/reorganization `docker build .` passes (fmt, lint, all tests, binary build).
clawbot added needs-review and removed needs-rework labels 2026-03-25 02:36:58 +01:00
clawbot commented 2026-03-25 02:40:05 +01:00
Author
Collaborator
Copy Link

Review: PR #41 (post-rework) — allowlist rename

Sneak's Feedback Verification

Feedback Item Status
Package internal/whitelist/ → internal/allowlist/ ✅ Done
Type HostWhitelist → HostAllowList ✅ Done
Constructor NewHostWhitelist() → New() (no stutter) ✅ allowlist.New(...)
Method IsWhitelisted() → IsAllowed() ✅ allowlist.IsAllowed(...)
All references in service.go updated ✅ Import, field, constructor call, method call, comments

Policy Compliance

Check Result
No linter/CI/test config changes ✅ .golangci.yml, Makefile, Dockerfile, .gitea/workflows/ untouched
No logic changes ✅ Purely rename — function bodies identical
Tests preserved with full assertions ✅ All test cases, assertions, expected values identical. External test style (package allowlist_test) correct
No stuttering names ✅ allowlist.New(), allowlist.HostAllowList, allowlist.IsAllowed()
Constructor naming ✅ New() per Go styleguide ("Constructors must be called New()")
Package doc comment ✅ Present
Old files removed ✅ internal/imgcache/whitelist.go and whitelist_test.go deleted
No scope creep ✅ Exactly 3 files changed

Docker Build

✅ fmt-check: pass
✅ lint: pass
✅ test: all packages pass (including internal/allowlist, internal/imgcache)
✅ binary build: pass
✅ final image: built successfully

Note on Remaining whitelist References

There are still whitelist references elsewhere in the codebase (config field names like WhitelistHosts, the unused Whitelist interface in imgcache.go, test helper names, error messages). These are all pre-existing and outside this PR's scope — the Whitelist interface was already dead code on main before this PR. A follow-up issue could address a broader rename if desired.

Verdict: PASS ✅

All of sneak's rework feedback addressed. Clean rename, no logic changes, no test weakening, no config modifications. Docker build passes.

## Review: PR #41 (post-rework) — allowlist rename ### Sneak's Feedback Verification | Feedback Item | Status | |---|---| | Package `internal/whitelist/` → `internal/allowlist/` | ✅ Done | | Type `HostWhitelist` → `HostAllowList` | ✅ Done | | Constructor `NewHostWhitelist()` → `New()` (no stutter) | ✅ `allowlist.New(...)` | | Method `IsWhitelisted()` → `IsAllowed()` | ✅ `allowlist.IsAllowed(...)` | | All references in `service.go` updated | ✅ Import, field, constructor call, method call, comments | ### Policy Compliance | Check | Result | |---|---| | No linter/CI/test config changes | ✅ `.golangci.yml`, `Makefile`, `Dockerfile`, `.gitea/workflows/` untouched | | No logic changes | ✅ Purely rename — function bodies identical | | Tests preserved with full assertions | ✅ All test cases, assertions, expected values identical. External test style (`package allowlist_test`) correct | | No stuttering names | ✅ `allowlist.New()`, `allowlist.HostAllowList`, `allowlist.IsAllowed()` | | Constructor naming | ✅ `New()` per Go styleguide ("Constructors must be called `New()`") | | Package doc comment | ✅ Present | | Old files removed | ✅ `internal/imgcache/whitelist.go` and `whitelist_test.go` deleted | | No scope creep | ✅ Exactly 3 files changed | ### Docker Build ``` ✅ fmt-check: pass ✅ lint: pass ✅ test: all packages pass (including internal/allowlist, internal/imgcache) ✅ binary build: pass ✅ final image: built successfully ``` ### Note on Remaining `whitelist` References There are still `whitelist` references elsewhere in the codebase (config field names like `WhitelistHosts`, the unused `Whitelist` interface in `imgcache.go`, test helper names, error messages). These are all **pre-existing** and outside this PR's scope — the `Whitelist` interface was already dead code on `main` before this PR. A follow-up issue could address a broader rename if desired. ### Verdict: **PASS** ✅ All of sneak's rework feedback addressed. Clean rename, no logic changes, no test weakening, no config modifications. Docker build passes.
clawbot added merge-ready and removed needs-review labels 2026-03-25 02:40:29 +01:00
clawbot removed their assignment 2026-03-25 02:40:29 +01:00
sneak was assigned by clawbot 2026-03-25 02:40:29 +01:00
sneak merged commit e34743f070 into main 2026-03-25 20:44:57 +01:00
sneak deleted branch refactor/extract-whitelist-package 2026-03-25 20:44:57 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#41
Delete Branch "refactor/extract-whitelist-package"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?