draft: remove deprecated positional logger from New and Handler - #71
Closed
harrydayexe wants to merge 7 commits into
Closed
harrydayexe wants to merge 7 commits into
harrydayexe wants to merge 7 commits into
Conversation
Drop the positional *slog.Logger parameter from server.New and server.Handler, completing the Phase 2 cleanup planned in #52. Callers now supply a logger exclusively via config.WithLogger; the fallback to slog.Default() is retained when no option is provided. All internal call sites, tests, doc examples, and README updated. The Phase-1 deprecated-path tests are removed. BREAKING CHANGE: server.New(logger, posts, cfg) is now server.New(posts, cfg); server.Handler(blog, logger, opts...) is now server.Handler(blog, opts...). Closes #55
harrydayexe
marked this pull request as draft
June 12, 2026 22:34
Brings v3-changes up to date with main (27 commits: feeds, SEO metadata, image support, health checks, cache control, the integration module). Conflict resolution: v3's `feat(server)!: remove deprecated positional logger from New and Handler` meets main's newer server code. - pkg/server/server.go: keep v3's logger-free Handler call and main's new AssetsDir option. - Test and doc call sites added on main still passed the positional logger to server.New and server.Handler; they now use the v3 signatures, with the logger supplied through config.WithLogger where a test depended on it. internal/server/command_test.go used discardLogger(), which v3 deleted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Split the goblog CLI out of the root module into a leaf module at ./cli so CLI dependencies are no longer inherited by library consumers. - cmd/goblog -> cli/cmd/goblog - internal/ -> cli/internal/ (already CLI-only; no pkg/ importers) - cli/go.mod replaces github.com/harrydayexe/GoBlog/v2 => ../, so the CLI always builds against the library at the same commit. The root module sheds urfave/cli/v3, fatih/color, mattn/go-colorable and mattn/go-isatty. pkg/... is untouched. The CLI generator tests loaded templates through a relative os.DirFS path into pkg/templates/default; they now use the embedded templates.Default, which does not depend on the tree layout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Dockerfile: download deps and build from ./cli; both go.mod files are copied up front so the `replace ../` resolves in the cached layer. - justfile: build/install/run recipes target the CLI module; test, vet, fmt, vulncheck and mod-tidy now iterate every module instead of stopping at the root module's `./...`. Coverage is reported per module because `go tool cover` resolves sources through its own module. - .goreleaser.yaml: `dir: cli`, shell completions generated in a before hook and shipped in every archive, and a `homebrew_casks` block that pushes to harrydayexe/homebrew-tap (`brews` is deprecated upstream). The matching CI workflow changes are held back in a separate commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- README: the `## CLI` section now covers `brew install harrydayexe/tap/goblog` and release archives; `go install` no longer works because the CLI module is not published. Adds fish completion and notes that Homebrew and the archives ship completions. - CONTRIBUTING: describe the three-module layout and why per-module commands are needed. - CLAUDE.md: record the module boundary and the new install channels. - version.go: the go install fallback path no longer applies. - justfile: `run-gen` invoked a `gen` subcommand that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes #93. Targets `v3-changes`. Moves the `goblog` CLI out of the root Go module into a leaf module at `./cli/`, and switches its distribution to a Homebrew tap plus release archives and Docker. `go install` is no longer a supported install path. > `v3-changes` was 27 commits behind `main`, so `main` was merged into it first (commit `a572885`, pushed directly to `v3-changes`). This PR sits on top of that merge. ## What changed **Module split** ``` /go.mod github.com/harrydayexe/GoBlog/v2 (library only) /cli/go.mod github.com/harrydayexe/GoBlog/v2/cli (never published) replace github.com/harrydayexe/GoBlog/v2 => ../ cmd/goblog/ moved from /cmd/goblog internal/ moved from /internal /integration/go.mod unchanged ``` `internal/` was already CLI-only — nothing under `pkg/` imported it — so the whole tree moved rather than being reached across the module boundary. The root module sheds `urfave/cli/v3`, `fatih/color`, `mattn/go-colorable` and `mattn/go-isatty`. `pkg/...` is untouched: `git diff v3-changes -- pkg/` is empty. **Build and release** - `.goreleaser.yaml` builds with `dir: cli`, generates bash/zsh/fish completions in a before hook, ships them in every archive, and publishes a `homebrew_casks` block to `harrydayexe/homebrew-tap`. The issue suggested `brews:`, but that is fully deprecated upstream as of GoReleaser v2.16 and makes `goreleaser check` fail, so this uses `homebrew_casks` instead. - `Dockerfile` builds from `./cli/cmd/goblog`. Both `go.mod` files are copied before `go mod download` so the `replace ../` resolves in the cached layer. - `justfile` recipes target the CLI module, and `test`/`test-race`/`vet`/`fmt`/`vulncheck`/`mod-tidy` now iterate every module instead of stopping at the root module's `./...`. Coverage is reported per module, because `go tool cover` resolves sources through the module it runs in. **Docs** README's `## CLI` section now documents `brew install harrydayexe/tap/goblog` and release archives. CONTRIBUTING describes the three-module layout; CLAUDE.md records the module boundary and the new install channels. ## Follow-up needed - **CI workflow changes are not in this PR** and must be applied separately — see the review comment below for the exact diff. Without them the CLI module is not vetted, tested or race-tested at all, and the release job has no `HOMEBREW_TAP_GITHUB_TOKEN`. - **The `harrydayexe/homebrew-tap` repo and its release token** still need creating, and `HOMEBREW_TAP_GITHUB_TOKEN` adding as a repository secret. Until then the `homebrew_casks` step will fail at release time. - **A final `go install`-capable release** with a deprecation notice, if you want one, as the issue suggests. ## Verification - `just test`, `just test-race`, `just vet`, `just fmt-check` and `addlicense -check` pass across all three modules. - `goreleaser check` passes; `goreleaser release --snapshot` produces archives containing the binary, LICENSE, README and `completions/`, and a cask with correct `binary` and `*_completion` stanzas. - The built binary reports the injected version and generates correctly. - Docker was unavailable in this environment, so the image build is verified by simulation — building `./cli/cmd/goblog` from a clean `git archive` of the tree, and running `go mod download` in `cli/` with only the two `go.mod`/`go.sum` pairs present. ## Note `README.md` claims GPL-3.0 in its badge and License section, but `LICENSE` is MPL 2.0 and every source header says MPL 2.0. Left alone as out of scope, but worth fixing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!--- START AUTOGENERATED NOTES ---> ### Changelog ([#95](#95)) #### 📚 Documentation - (cli) document Homebrew and archive installs, drop go install #### ♻️ Refactoring - (cli) move CLI into a ./cli leaf module #### 🏗️ Build System - (cli) build the CLI module and publish a Homebrew cask #### 🤖 CI - add updates for split cli and homebrew tap <!--- END AUTOGENERATED NOTES --->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Drop the positional *slog.Logger parameter from server.New and server.Handler, completing the Phase 2 cleanup planned in #52. Callers now supply a logger exclusively via config.WithLogger; the fallback to slog.Default() is retained when no option is provided.
All internal call sites, tests, doc examples, and README updated. The Phase-1 deprecated-path tests are removed.
BREAKING CHANGE: server.New(logger, posts, cfg) is now server.New(posts, cfg); server.Handler(blog, logger, opts...) is now server.Handler(blog, opts...).
Closes #55
Changelog (#71)
📚 Documentation
♻️ Refactoring
🏗️ Build System
🤖 CI
❓ Uncategorised!