[codex] complete #631 #637 #643 #644#682
Conversation
github-actions
Bot
added
compiler
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38a8d884a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dynamic.MaxURLs = parseInt(field.Value) | ||
| case "CacheSeconds": | ||
| dynamic.CacheSeconds = parseInt(field.Value) |
There was a problem hiding this comment.
When DynamicSitemap.MaxURLs or CacheSeconds are computed from a constant/function instead of a literal, parseInt returns 0 but parseSEOAddon still reports success, so LoadConfigFile never falls back to the executable config loader. In that scenario the generated sitemap handler silently uses the default max URL cap and disables successful-response caching instead of the values the Go config would produce. Please detect non-literal integer expressions here and return false, matching the fallback behavior used for BaseURL, Disallow, and ExtraURLs.
Useful? React with 👍 / 👎.
| if !strings.HasPrefix(loc, "/") { | ||
| return "", fmt.Errorf("sitemap URL %q must be absolute or root-relative", loc) | ||
| } | ||
| return AbsoluteURL(base, loc), nil |
There was a problem hiding this comment.
When a dynamic provider returns a root-relative URL that includes a query or fragment, such as /products?page=2, this branch passes the original string to AbsoluteURL as a path, so the generated <loc> becomes https://example.com/products%3Fpage=2 rather than a normalized URL. Absolute URLs already strip query/fragment via the parsed URL above, so please use the parsed root-relative URL components here (and in the mirrored build-time extra-URL path) instead of joining the raw loc.
Useful? React with 👍 / 👎.
| if generatedDynamicSitemapEnabled(options) { | ||
| imports["gowdkseo"] = "github.com/cssbruno/gowdk/runtime/seo" | ||
| imports["gowdkseositemap"] = strings.TrimSpace(options.Sitemap.Dynamic.ImportPath) | ||
| } |
There was a problem hiding this comment.
When DynamicSitemap.ImportPath points at a package inside the user's app module and there are no other app-owned backend imports, this generated import is not included in appBackendImportPaths, so moduleSource does not add the app module require/replace needed by the generated app. The documented dynamic sitemap example in a normal external app module will then fail at generated-app go mod tidy/build because the provider import cannot be resolved. Please add the sitemap provider import path to the module-dependency scan as well as to the generated source imports.
Useful? React with 👍 / 👎.
| Paths: append([]string(nil), options.Paths...), | ||
| } | ||
| if err := runInWorkingDir(cli.ProjectRoot, func() error { | ||
| return runTestBuildOnce(cli, request, options.JSON) |
There was a problem hiding this comment.
When gowdk test is invoked with --config pointing at a subdirectory plus explicit .gwdk file paths relative to the original working directory, these paths are copied into the build request and then the build runs after chdir to cli.ProjectRoot. buildOnce passes explicit paths straight to lang.ParseBuildFiles, so a path like app/pages/home.gwdk is looked up as app/app/pages/home.gwdk and the test build fails even though the same arguments work for gowdk build. Normalize explicit paths to absolute paths (or rebase them to the project root) before calling runInWorkingDir.
Useful? React with 👍 / 👎.
cssbruno
force-pushed
the
codex/refactor-open-issues
branch
from
38a8d88 to
c1ceeef
Compare
1 participant
Summary
Closes #631.
Closes #637.
Closes #643.
Closes #644.
jsonldpage metadata, validation diagnostics, deterministic JSON-LD rendering, manifest/build-report metadata, and an SEO runtime sitemap handler with an optional generated app provider hook.docs-site/dist/sitebuild-only output, moves production docs-site build/smoke steps into scripts, and updates Render/CI to build the real site instead of a placeholder.Verification
go test ./runtime/seogo test ./internal/parser ./internal/compilergo test ./internal/buildgengo test ./internal/appgen ./internal/project(cd docs-site && go test ./cmd/syncdocs)scripts/check-docs-style.shscripts/check-docs-links.shscripts/check-removed-syntax.shscripts/check-doc-versions.shgo run ./cmd/gowdk build --config examples/seo/gowdk.config.go --out /tmp/gowdk-seo-build --app /tmp/gowdk-seo-app --bin /tmp/gowdk-seo-site examples/seo/*.gwdk/tmp/gowdk-seo-sitesitemap dynamic URL merge(cd docs-site && scripts/build-production.sh && scripts/smoke-production.sh)(cd docs-site && go test ./... && go vet ./...)go build ./cmd/gowdkgo test ./...scripts/test-go-modules.shgit diff --checkNote: the docs-site production build currently prints existing non-blocking
audit_bundle_secretwarnings for documentation examples in non-production mode; the build and smoke passed.