[ajitpratap0]

Summary

Three low-severity review notes ahead of v1.16.0.

1. One definition of the L001-L010 rule set. New rules.CoreRules(cfg rules.CoreConfig) []linter.Rule and rules.DefaultCoreConfig() in pkg/linter/rules (above pkg/linter, so no import cycle with the rule sub-packages). These now build from it: gosqlx lint (createLinter), gosqlx action (builtinRules), the MCP lint_sql tool (newFullLinter), the C binding's gosqlx_lint (newLinter), and the first ten rules of gosqlx.Lint (allRules). The "keep the three in sync" comments are gone.
   • Behaviour is unchanged. CoreConfig covers the settings that differ between call sites: the L003 blank-line limit, the L004 depth and indent size, and the L005 line length. lint, MCP and cbinding use the CLI defaults (1, 4/4, 100, or the caller's max length). gosqlx.Lint uses L005=120. action keeps its own lenient settings (2, 10/4, 120).
   • Violation order follows rule order, and the linter does not sort. CoreRules keeps the existing grouped order (L001-L005, L010, L006, L008, L009, L007). action used numeric order, so it sorts by ID to keep that order.
2. The CLI version comes from the library. cmd.Version now defaults to gosqlx.Version, and -ldflags -X .../cmd.Version=<tag> can still override it (checked: a plain build prints 1.14.0, and -X ...=9.9.9-test prints 9.9.9-test). The unused Version: "1.14.0" literal in rootCmd is removed. The near-tautological version test is replaced with one that runs --version and checks the output contains gosqlx.Version. release.yml gains a step, right after checkout, that fails when the tag without its leading v differs from const Version in pkg/gosqlx/gosqlx.go.
3. README dialect note. bigquery and redshift were described as having "keyword recognition and basic parsing only". In fact DialectKeywords returns nil for both, so they use the generic keyword set, and they only have capability-gated grammar (for example BigQuery's QUALIFY, UNNEST in FROM and (struct).field; each was checked with gosqlx validate --dialect bigquery). No other docs or website pages repeated the sentence.

Test Plan

• task check (fmt, vet, golangci-lint, race tests)
• task test:cbinding
• go test ./pkg/mcp/... ./cmd/...
• New pkg/linter/rules/core_test.go: exactly L001..L010 once each in the documented order, L005 config applied, DefaultCoreConfig values pinned, fresh instances per call
• Built the CLI with and without -ldflags -X and ran --version
• Ran the release guard's shell logic locally (v1.14.0 passes, v1.15.0 fails against main's const)

Breaking Changes

None. One new exported API: package github.com/ajitpratap0/GoSQLX/pkg/linter/rules (CoreRules, CoreConfig, DefaultCoreConfig). Unstamped CLI builds now report gosqlx.Version. On main both values are 1.14.0, so nothing visible changes.

Note: this conflicts with #845 in cmd/gosqlx/cmd/root.go. To resolve, keep var Version = gosqlx.Version, drop the rootCmd Version: literal, and bump only gosqlx.Version.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RkzVu3JY9Y6BRWfjesWts9

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
gosqlx-website	Ignored	Preview	Oct 22, 2026 11:04am UTC

[github-actions]

Claude finished @ajitpratap0's task in 33s —— View job

---

Review of PR #853

I read the diff but did not run the tests (no run tooling was available). I found no blocking issues, and the refactor looks behaviour-preserving.

Lint rule consolidation

• rules.CoreRules and CoreConfig are clean. Putting them above pkg/linter avoids the import cycle, and each call returns fresh instances.
• The ordering is handled correctly. action sorts by ID to keep its old order, and gosqlx.Lint keeps the grouped order.
• allRules() in pkg/gosqlx/lint.go copies DefaultCoreConfig() and then overrides L005 to 120. append(CoreRules(...), ...) is safe because the slice is freshly allocated.
• Minor: builtinRules sorts with strings.Compare on IDs. That is lexicographic, which is fine for L001–L010 because the IDs are zero-padded. If rules past L099 are ever added there, it would still sort correctly.
• Minor: CoreRules ignores the L006–L009 settings. That matches the doc comment, but a future call site that needs a different comma style or case rule will have to extend CoreConfig.

CLI version

• var Version = gosqlx.Version keeps the -ldflags -X override working. -X only overrides string variables initialised with a constant, and gosqlx.Version is a const, so this works.
• The new test runs --version through the shared rootCmd. It resets the output and args in t.Cleanup, which is good. The rootCmd.Version field is set in init(), so it is already populated when the test runs.
• The long doc comment above Version in root.go still lists v1.14.0-era features. This is cosmetic only.

Release guard (.github/workflows/release.yml)

• The sed pattern matches the current line, const Version = "1.14.0" at pkg/gosqlx/gosqlx.go:33. It would silently stop matching if the const were moved into a const (...) block, but the empty-string check turns that into a loud failure, which is the right outcome.
• github.ref_name goes through env, not direct ${{ }} interpolation in the script. That avoids script injection.
• The guard will fail any tag pushed before the const is bumped. That is the intent, and it fits the documented release flow where the docs PR merges before tagging. Make sure the release checklist says to bump gosqlx.Version.

Tests

• core_test.go pins the rule order, the L005 config and the default values, which is good coverage. The CLI, MCP and cbinding call sites are only covered indirectly by their existing tests, and you ran those.

README: the corrected bigquery and redshift wording matches the DialectKeywords behaviour you describe.

Overall this looks good to merge. Resolve the #845 conflict as you described.

[github-actions]

Claude finished @ajitpratap0's task in 20s —— View job

---

Review of PR #853

I read the diff against origin/main but did not run the tests or build. I found no blocking issues. The refactor looks behaviour-preserving.

Lint rule consolidation

• rules.CoreRules and CoreConfig in pkg/linter/rules/core.go are clean. Placing them above pkg/linter avoids the import cycle, and every call returns fresh instances.
• Rule order is preserved. gosqlx.Lint keeps the grouped order. action sorts by ID with slices.SortFunc to keep its numeric order. The sort is lexicographic, which is correct for the zero-padded L001–L010 IDs.
• allRules() in pkg/gosqlx/lint.go overrides L005 to 120 on a copy of the defaults. append onto the freshly allocated slice is safe.
• Minor: CoreConfig does not cover L006–L009, as the doc comment says. A call site that needs a different comma style or keyword case would have to extend the struct.
• Minor: actionCoreConfig is a package-level var. It is passed by value, so it cannot be mutated through CoreRules. A function or a comment saying it must not be modified would make that explicit, but this is optional.

CLI version

• var Version = gosqlx.Version keeps the -ldflags -X override working, because -X can set a string variable initialised from a constant.
• Dropping the rootCmd Version: literal is fine, since init() assigns rootCmd.Version = Version.
• The long doc comment above Version in root.go still lists v1.14.0-era features. This is cosmetic.

Release guard (.github/workflows/release.yml)

• github.ref_name goes through env, which avoids script injection.
• The sed pattern requires const Version = "..." at the start of a line. If the const moves into a const (...) block, the empty-string check turns the mismatch into a loud failure.
• The guard fails any tag pushed before gosqlx.Version is bumped. That matches the intent, but the release checklist in CLAUDE.md doesn't mention the bump yet. Adding that step there would help.

Tests

• core_test.go pins the rule order, the L005 config, the default values and fresh instances per call.
• The CLI, MCP and cbinding call sites are only covered indirectly by their existing tests, which you ran.

README: the corrected bigquery and redshift wording matches the DialectKeywords behaviour you describe.

Overall this looks good to merge. Resolve the #845 conflict in root.go as described in the PR body.