GoBlog V3 - #104
Draft
harrydayexe wants to merge 17 commits into
Draft
GoBlog V3#104harrydayexe wants to merge 17 commits into
harrydayexe wants to merge 17 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
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 --->
WithBaseOption was deprecated in favour of BaseOption.AsGeneratorOption, which all call sites already use. Remove it now that the public API can break, and cover the replacement method directly in pkg/config. BREAKING CHANGE: config.WithBaseOption has been removed. Call BaseOption.AsGeneratorOption() on the base option instead, e.g. config.WithLogger(logger).AsGeneratorOption(). Refs #57 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With WithBaseOption gone, BaseOption.AsGeneratorOption and its siblings are the only way to pass a BaseOption to a constructor. The package overview described BaseOption flowing into the specialised option types but never named the methods that do the lifting, so add them to the "Option types" section with a worked example for each. Refs #57 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The godoc examples for WithBlogRoot and WithLogger passed a BaseOption straight into generator.New and watcher.New, which only accept GeneratorOption and WatcherOption. With WithBaseOption removed, AsGeneratorOption and AsWatcherOption are the only route, so the examples now show them and compile as written. Also correct the WithBlogRoot doc comment, which said it returns an "Option" rather than a BaseOption. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Server.New now accepts `...config.ServerOption` instead of a `config.ServerConfig` value, and the resolved values live in a `config.ServerConfig` embedded in the Server. ServerConfig is no longer a container of option slices: it holds the resolved BlogRoot, Port, Host, Logger, CacheControlTTL, HealthChecks, AssetsDir and TemplateDir values, plus the middleware chain and the generator/renderer options forwarded to the internal generator and template renderer. BaseServerOption is renamed ServerOption to match GeneratorOption, RendererOption and WatcherOption. Generator and renderer options reach the server through new AsServerOption methods, and the template filesystem through the new WithTemplateDir option. BREAKING CHANGE: server.New takes variadic config.ServerOption values, config.BaseServerOption is renamed to config.ServerOption, and config.ServerConfig no longer carries Server, Gen, TemplateDir and RendererOpts option slices. Refs #72 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Assert that New resolves defaults and every server option into the embedded config.ServerConfig, and that generator, renderer and template directory options supplied as ServerOption values reach the generator and template renderer the server builds. Refs #72 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rewrite the server, config and README examples for the new server.New(posts, opts...) signature, and describe how base, generator and renderer options are converted with AsServerOption. Refs #72 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The server forwards its resolved blog root to the generator, so the option only needs supplying once. Refs #72 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The CLI passes --root-path as a server option only and relies on the server forwarding its resolved blog root to the generator, but nothing asserted that forwarding. Existing blog-root tests only exercise routing, which is driven by the server's own BlogRoot, so dropping the forwarding would silently emit wrong in-page link and image paths with every test still green. Assert the forwarded field in TestNew_OptionsResolveIntoServerConfig and add TestNew_BlogRootForwardedToGenerator, which serves a post under a custom blog root and checks the rendered page data and the rewritten relative image path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes #57 > [!NOTE] > This pull request was written by [rain](https://github.com/harrydayexe/rain) running Claude Code. It has not been reviewed by a human. CI and an automated review pass run before it is handed over. > > It targets `v3-changes` rather than the default branch: `57-remove-deprecated-withbaseoption-wrapper-function` was already linked to the issue, and its history says it was cut from `v3-changes`. ## Issue [#57: Remove deprecated WithBaseOption wrapper function](#57) ## What the agent reports Pushed. Branch is two commits ahead of `v3-changes`. ## What changed **`78c3c5a` — the removal (pre-existing on the branch, verified not re-done).** Deletes `func WithBaseOption(BaseOption) GeneratorOption` from `pkg/config/generatorOption.go` and adds `pkg/config/generatorOption_test.go` covering the replacement `BaseOption.AsGeneratorOption()` — that it lifts logger, blog root and assets dir, and that it leaves the generator-specific func pointers nil so the `else if` application chain can't pick a field the caller never set. **`204588f` — doc follow-up I added.** `pkg/config/doc.go`'s "Option types" section said `BaseOption` flows into the specialised types "via the embedded BaseOption" but never named the methods that do the lifting. With the wrapper gone, `As*Option()` is the *only* route, so the package overview described a capability with no stated way to reach it. Added a short paragraph naming `AsGeneratorOption` / `AsWatcherOption` / `AsServerOption` with one example each. This is the `CLAUDE.md` rule that API changes carry doc updates; I kept it to the one surface the removal actually invalidated. ## Verification - `grep` for `WithBaseOption` across all `*.go` and `*.md`: no matches anywhere in the repo, so no dangling call sites or stale docs. - Per CI (`.github/workflows/test.yml` runs vet + test + `-race` per module): `go vet ./...`, `go test ./...` and `go test -race ./...` all pass for both `.` and `cli`. `go vet ./...` passes for `integration` (its tests need Docker and were not run). - `gofmt -l .` clean. - `go doc ./pkg/config` renders the new section correctly. - New tests run and pass under `-count=1` (not just cached). - `addlicense -check` could not run — the binary isn't installed in this environment. The new test file carries the MPL header matching its siblings, and `doc.go` was edited in place, so the license check should pass. ## For the reviewer Two things worth a look: **The doc examples are hand-verified, not compiled.** I initially wrote `server.New(cfg, config.WithLogger(logger).AsServerOption())`, which is wrong — `server.New` takes `(posts fs.FS, opts config.ServerConfig)`, not variadic options. I checked the real signatures and corrected it to construct a `ServerConfig` with a `Server: []config.BaseServerOption{...}` field. The `generator.New` and `watcher.New` examples match their actual signatures. Since these live in comments nothing enforces them, so a second read is worthwhile. **Scope judgement.** The issue names only `WithBaseOption`. I did not touch the sibling `As*Option` methods or the `AsOption()` methods on the individual config types. The doc paragraph is the sole addition beyond the literal removal. The commit message on `78c3c5a` already carries a `BREAKING CHANGE:` footer, which matters if release tooling derives the v3.0.0 notes from commit trailers. <!--- START AUTOGENERATED NOTES ---> ### Changelog ([#102](#102)) #### 📚 Documentation - (config) document As*Option lifting in package docs - (config) lift base options in godoc examples #### ❓ Uncategorised! - feat(config)!: remove deprecated WithBaseOption wrapper <!--- END AUTOGENERATED NOTES --->
…erConfig (#72) (#103) Closes #72 > [!NOTE] > This pull request was written by [rain](https://github.com/harrydayexe/rain) running Claude Code. It has not been reviewed by a human. CI and an automated review pass run before it is handed over. > > It targets `v3-changes` rather than the default branch: `72-make-server-take-a-variadic-array-of-options-and-store-config-in-serverconfig` was already linked to the issue, and its history says it was cut from `v3-changes`. ## Issue [#72: Make Server take a variadic array of Options and store config in ServerConfig](#72) ## What the agent reports Pushed. Four commits on the branch, baselined on `v3-changes`. ## What changed **`pkg/config`** - `ServerConfig` is now the *resolved* server configuration — `BlogRoot`, `Port`, `Host`, `Logger`, `CacheControlTTL`, `HealthChecks`, `AssetsDir`, `TemplateDir`, plus `Middleware`, `GeneratorOpts`, `RendererOpts`. It no longer carries `Server`/`Gen`/`TemplateDir`/`RendererOpts` option slices. - `BaseServerOption` → `ServerOption`, matching `GeneratorOption`/`RendererOption`/`WatcherOption`. - New plumbing so everything the old struct fields carried can travel in the variadic list: `GeneratorOption.AsServerOption()`, `RendererOption.AsServerOption()`, and a new `TemplateDir` config type with `WithTemplateDir(fs.FS)` (+ `AsOption`). `BaseOption.AsServerOption()` is unchanged. **`pkg/server`** - `func New(posts fs.FS, opts ...config.ServerOption) (*Server, error)`; `Server` embeds `config.ServerConfig` instead of seven separate config types and three unexported deferred-init fields. Field promotion means `srv.Port`, `srv.Logger.Logger`, `srv.HealthChecks.Enabled` etc. still read the same way. **`cli/internal/server`** — builds a `[]config.ServerOption`; `runServe` now takes `(ctx, postsPath, posts, watch, opts...)`. I dropped the duplicate blog-root generator option there (and in one test): `initialize` assigns `s.generator.BlogRoot = s.BlogRoot` unconditionally, so applying it twice was a no-op. **Tests** — all existing call sites migrated (`pkg/server`, `cli/internal/server`, `integration`). New `pkg/server/options_test.go` covers defaults, every option resolving into the embedded `ServerConfig`, health-checks deferring generator construction, and generator/renderer/template-dir options reaching the internal generator and renderer (the last renders a custom template through a forwarded `WithFuncs` entry). These fail without the change — the APIs they use don't exist on the old signature. **Docs** — `pkg/server/doc.go` (new "# Configuration" section), `pkg/config/doc.go`, the option godoc examples, and README examples all rewritten for the variadic form; added a short serve example to the library section of README. ## Verification `go vet ./...`, `go test ./...` and `go test -race ./...` for both `.` and `cli`; `go vet ./...` for `integration`; `gofmt -l` clean; `go mod tidy` a no-op in all three modules. I did **not** run the integration suite (`just test-integration`) — it needs Docker; those tests compile and were updated. A CLI smoke run was blocked by a permission prompt, so end-to-end serve coverage rests on the CLI unit tests (index, post, blog-root, watch-reload, feeds). ## For review - The rename `BaseServerOption` → `ServerOption` and the `ServerConfig` redefinition are the breaking parts; commit 1 is marked `feat(server)!` with a `BREAKING CHANGE:` footer. Worth confirming this is the naming you want for v3 given the issue wrote `config.ServerOptions`. - `New` appends the server's own logger to `ServerConfig.GeneratorOpts`, so that slice holds one more option than the caller passed. - `GeneratorOption.AsServerOption` shadows the `AsServerOption` promoted from its embedded `BaseOption`; `config.WithLogger(l).AsGeneratorOption().AsServerOption()` therefore routes to the generator, while `config.WithLogger(l).AsServerOption()` configures the server (which forwards it anyway). <!--- START AUTOGENERATED NOTES ---> ### Changelog ([#103](#103)) #### 📚 Documentation - document the variadic server options API #### 🧪 Tests - (server) cover variadic option plumbing - (server) drop redundant generator blog root option - (server) cover blog root forwarding to the generator #### ❓ Uncategorised! - feat(server)!: take variadic options and store resolved config <!--- 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.
No description provided.