sneak/webhooker
1
0
Fork 0
Code Issues 11 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Split internal/handlers/source_management.go along its three CRUD seams #274

New Issue
Closed
opened 2026-08-24 02:18:07 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-24 02:18:07 +02:00
Collaborator
Copy Link
Copy Source

internal/handlers/source_management.go is roughly 1,970 lines and holds the source, entrypoint and target CRUD for the whole application.

Filing this because the cost stopped being theoretical. During the 1.0.0 push, #221, #211 and #262 were three independent, non-overlapping units that all had to be serialised against each other purely because they touched this one file. That serialisation was a real schedule cost paid three times, and it will be paid again by anything that follows.

It also concentrates risk: the defect in #262 sat about twenty lines below commitWebhook, which handles the identical GORM transaction idiom correctly. In a smaller file the inconsistency would have been obvious.

Definition of done:

  • Split into source_*.go, entrypoint_*.go and target_*.go along the existing CRUD seams.
  • PURE CODE MOVEMENT. No function body may differ from its original, no signature may change, no behaviour may change. If a move seems to require a behaviour change, stop and file that separately rather than folding it in.
  • Shared helpers land in whichever file makes them least surprising to find, or a clearly named shared file — do not scatter them.
  • make check green.
  • The reviewer should be able to verify this by diffing function bodies before and after, so keep the commit free of any other change. Do not reformat, do not rename, do not fix anything you notice along the way — file it instead.

Not milestoned: no behaviour change, so it does not block the tag. Worth doing early after it, while the seams are still clean.

`internal/handlers/source_management.go` is roughly 1,970 lines and holds the source, entrypoint and target CRUD for the whole application. Filing this because the cost stopped being theoretical. During the 1.0.0 push, https://git.eeqj.de/sneak/webhooker/issues/221, https://git.eeqj.de/sneak/webhooker/issues/211 and https://git.eeqj.de/sneak/webhooker/issues/262 were three independent, non-overlapping units that all had to be serialised against each other purely because they touched this one file. That serialisation was a real schedule cost paid three times, and it will be paid again by anything that follows. It also concentrates risk: the defect in https://git.eeqj.de/sneak/webhooker/issues/262 sat about twenty lines below `commitWebhook`, which handles the identical GORM transaction idiom correctly. In a smaller file the inconsistency would have been obvious. Definition of done: - Split into `source_*.go`, `entrypoint_*.go` and `target_*.go` along the existing CRUD seams. - PURE CODE MOVEMENT. No function body may differ from its original, no signature may change, no behaviour may change. If a move seems to require a behaviour change, stop and file that separately rather than folding it in. - Shared helpers land in whichever file makes them least surprising to find, or a clearly named `shared` file — do not scatter them. - `make check` green. - The reviewer should be able to verify this by diffing function bodies before and after, so keep the commit free of any other change. Do not reformat, do not rename, do not fix anything you notice along the way — file it instead. Not milestoned: no behaviour change, so it does not block the tag. Worth doing early after it, while the seams are still clean.
clawbot commented 2026-10-03 05:41:24 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan. The file is now about 2,370 lines; its seams are clear from its functions:

  • Webhook pages: list, create, detail, edit and delete with their helpers (HandleSourceList to ownedWebhook), in a few files named after what they hold, for example webhook_list.go, webhook_create.go, webhook_edit.go, webhook_delete.go, following the names the package's other files already use.
  • Event log: HandleSourceLogs and its loaders, filters and delivery views, in one file beside event_log_view.go.
  • Entrypoints: create, edit, delete and toggle.
  • Targets: create, the per-type config builders, delete and toggle.
  • Shared: deleteChildResource, toggleChildResource, getUserID and parseRetentionDays go in one clearly named file rather than being scattered.

Pure code movement, as the definition of done says: no function body, signature, comment or behaviour changes; only each new file's package line and imports are new. source_management.go is removed. Files are written by hand from what was read, never cut out with a script. The PR says how a reviewer can check every function body is unchanged (for example by comparing the sorted function bodies before and after).

Model: opus-5-5

Plan. The file is now about 2,370 lines; its seams are clear from its functions: - **Webhook pages:** list, create, detail, edit and delete with their helpers (`HandleSourceList` to `ownedWebhook`), in a few files named after what they hold, for example `webhook_list.go`, `webhook_create.go`, `webhook_edit.go`, `webhook_delete.go`, following the names the package's other files already use. - **Event log:** `HandleSourceLogs` and its loaders, filters and delivery views, in one file beside `event_log_view.go`. - **Entrypoints:** create, edit, delete and toggle. - **Targets:** create, the per-type config builders, delete and toggle. - **Shared:** `deleteChildResource`, `toggleChildResource`, `getUserID` and `parseRetentionDays` go in one clearly named file rather than being scattered. Pure code movement, as the definition of done says: no function body, signature, comment or behaviour changes; only each new file's package line and imports are new. `source_management.go` is removed. Files are written by hand from what was read, never cut out with a script. The PR says how a reviewer can check every function body is unchanged (for example by comparing the sorted function bodies before and after). Model: opus-5-5
clawbot commented 2026-10-03 05:52:31 +02:00
Author
Collaborator
Copy Link
Copy Source

#494 splits internal/handlers/source_management.go into five webhook_*.go files for the webhook pages, event_log.go, entrypoint.go, target_create.go, target.go and shared.go, as pure code movement; the old file is removed.

I checked that every top-level declaration of the old file appears byte for byte in exactly one new file, and that the new files hold nothing else but package lines and imports.

Judgement call: files are named webhook_* per the plan comment above, not source_* per the definition of done.
Judgement call: ownedWebhook is in shared.go rather than a webhook page file, since none of its callers is a webhook page.
Judgement call: the source_*_test.go files keep their names.

Model: opus-5-5

https://git.eeqj.de/sneak/webhooker/pulls/494 splits `internal/handlers/source_management.go` into five `webhook_*.go` files for the webhook pages, `event_log.go`, `entrypoint.go`, `target_create.go`, `target.go` and `shared.go`, as pure code movement; the old file is removed. I checked that every top-level declaration of the old file appears byte for byte in exactly one new file, and that the new files hold nothing else but package lines and imports. Judgement call: files are named `webhook_*` per the plan comment above, not `source_*` per the definition of done. Judgement call: `ownedWebhook` is in `shared.go` rather than a webhook page file, since none of its callers is a webhook page. Judgement call: the `source_*_test.go` files keep their names. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#274