Skip to content

Conversation

@osteele
Copy link
Owner

@osteele osteele commented Sep 1, 2025

Fix: Modernize build tooling and add pre-commit hooks

Background

The lack of consistent formatting in the main branch was leading to gratuitous differences between branches, making it difficult to review actual changes. This PR modernizes the build tooling and adds automatic formatting checks to prevent this issue going forward. The plan is to merge this to main first, which will unlock several bug fix and feature branches that can then be rebased on top of it.

Summary

  • Modernized Makefile with organized targets and better UX
  • Updated golangci-lint config to v2 format with best practices
  • Added tools.go for proper tool dependency management
  • Updated developer documentation to match new tooling
  • Added pre-commit hooks for automatic code quality checks
  • All existing lint issues have been fixed

Makefile Improvements

  • Reorganized targets into logical categories (Development, Testing, Code Quality, etc.)
  • Added comprehensive help system with target descriptions
  • Implemented colored output for better visibility
  • Added modern targets: coverage reports, benchmarks, dependency management
  • Added safety checks for required tools
  • Improved error messages and user feedback
  • Added make install-hooks, make run-hooks, and make update-hooks for pre-commit management

Linter Configuration (golangci-lint v2)

  • Added version field required for v2 compatibility
  • Fixed output format from array to map structure
  • Removed deprecated linters (typecheck, gosimple, exportloopref)
  • Replaced exportloopref with copyloopvar (new name in v2)
  • Moved goimports and gofmt from linters to formatters
  • Added comprehensive linter settings for better code quality
  • Configured appropriate exclusions for tests and generated code
  • Added limits on issue reporting to reduce noise
  • All lint issues resolved - project now passes make lint with 0 issues

Tool Management

  • Created tools.go with build tag for tool dependencies
  • Tools are now tracked in go.mod for version consistency
  • Added installation instructions for tools that can't be vendored
  • Documented golangci-lint v1.x compatibility issues with Go 1.23

Pre-commit Hooks (New)

  • Added .pre-commit-config.yaml with Go-specific hooks
  • Automatic checks on commit: go fmt, go vet, go mod tidy, golangci-lint, go build
  • General code quality checks: trailing whitespace, file endings, YAML syntax, large files
  • Auto-fixes common issues like whitespace and missing newlines
  • Hooks run automatically after make install-hooks setup

Documentation Updates

  • Updated CONTRIBUTING.md with new make targets
  • Clarified tool installation process
  • Added comprehensive testing and linting instructions
  • Documented code generation workflow
  • Organized commands by category for better discoverability
  • Added instructions for setting up git hooks with pre-commit

GitHub Workflows Updates

  • Updated go.yml to use Makefile targets for consistency
  • Standardized on Go 1.23.x (latest version only)
  • Added coverage reporting job with Codecov integration
  • Added go.mod verification job
  • Updated golangci-lint.yml with version specification (v2.4)
  • Added permissions and configuration options
  • Created new ci.yml workflow for comprehensive CI checks
  • Added format checking workflow to ensure code formatting

Breaking Changes

None - all existing workflows continue to work

Testing

All Makefile targets tested and working:

  • ✅ make help - displays organized help
  • ✅ make fmt - formats code
  • ✅ make lint - runs linter (0 issues)
  • ✅ make test - runs tests
  • ✅ make build - builds binary
  • ✅ make vet - runs go vet
  • ✅ make tools - installs development tools
  • ✅ make install-hooks - sets up pre-commit hooks
  • ✅ make run-hooks - all hooks passing

Notes

  • golangci-lint is installed separately as a standalone binary (standard practice for Go projects)
  • Pre-commit automatically runs formatting and linting on every commit
  • Tests excluded from pre-commit due to environment issues (run via make test)
  • S1005 exclusion applied only to specific test file (values/drop_test.go) where needed

Checklist

  • I have read the contribution guidelines.
  • make test passes.
  • make lint passes.
  • New and changed code is covered by tests.
  • Performance improvements include benchmarks.
  • Changes match the documented (not just the implemented) behavior of Shopify.

@osteele osteele requested a review from Copilot September 1, 2025 02:12

This comment was marked as outdated.

@osteele osteele changed the title Fix/lint errors Modernize build tooling, fix lint and format errors, and add pre-commit hooks Sep 1, 2025
@osteele osteele force-pushed the fix/lint-errors branch 3 times, most recently from efea8fe to 5184313 Compare September 1, 2025 03:06
@osteele osteele requested review from Copilot and danog September 1, 2025 03:10
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modernizes the build tooling infrastructure by upgrading to golangci-lint v2, adding pre-commit hooks for automated code quality checks, and organizing the Makefile with comprehensive development targets. The changes focus on improving developer experience and maintaining consistent code quality across the project.

Key changes:

  • Updated golangci-lint configuration to v2 format with curated linter settings
  • Added comprehensive pre-commit hooks for automatic formatting, linting, and quality checks
  • Reorganized Makefile with categorized targets, colored output, and modern development workflows

Reviewed Changes

Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
values/drop_test.go Fixed linter warning by removing unused assignment operator
values/convert.go Removed unnecessary blank line for code formatting consistency
tools.go Added tool dependency management with build tags for code generation tools
render/file_template_store.go Added security exclusion comment for trusted file path usage
go.mod Updated dependencies including testify and added golang.org/x/tools
expressions/scanner.go Added code generation header to exclude from manual editing
expressions/parser.go Updated code generation command to properly add generation header
cmd/liquid/main.go Added nolint directives for intentional stderr writes in CLI usage
PR_COMMENT.md Added comprehensive documentation of all changes and improvements
Makefile Complete rewrite with organized targets, help system, and modern workflows
CONTRIBUTING.md Updated documentation to match new tooling and development workflow
.pre-commit-config.yaml Added pre-commit configuration with Go-specific and general quality hooks
.golangci.yml Migrated to v2 format with curated linter settings and exclusions
.github/workflows/golangci-lint.yml Updated to use golangci-lint v2.4.0 with enhanced configuration
.github/workflows/go.yml Modernized to use Makefile targets and added coverage reporting
.github/workflows/ci.yml Added comprehensive CI workflow for format checking and quality gates

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

osteele and others added 6 commits November 6, 2025 17:27
## Summary
- Modernized Makefile with organized targets and better UX
- Updated golangci-lint config to v2 format with best practices
- Added tools.go for proper tool dependency management
- Updated developer documentation to match new tooling

## Makefile Improvements
- Reorganized targets into logical categories (Development, Testing, Code Quality, etc.)
- Added comprehensive help system with target descriptions
- Implemented colored output for better visibility
- Added modern targets: coverage reports, benchmarks, dependency management
- Added safety checks for required tools
- Improved error messages and user feedback

## Linter Configuration (golangci-lint v2)
- Added version field required for v2 compatibility
- Fixed output format from array to map structure
- Removed deprecated linters (typecheck, gosimple, exportloopref)
- Replaced exportloopref with copyloopvar (new name in v2)
- Moved goimports and gofmt from linters to formatters
- Added comprehensive linter settings for better code quality
- Configured appropriate exclusions for tests and generated code
- Added limits on issue reporting to reduce noise

## Tool Management
- Created tools.go with build tag for tool dependencies
- Tools are now tracked in go.mod for version consistency
- Added installation instructions for tools that can't be vendored
- Documented golangci-lint v1.x compatibility issues with Go 1.23

## Documentation Updates
- Updated CONTRIBUTING.md with new make targets
- Clarified tool installation process
- Added comprehensive testing and linting instructions
- Documented code generation workflow
- Organized commands by category for better discoverability

## GitHub Workflows Updates
- Updated go.yml to use Makefile targets for consistency
- Standardized on Go 1.23.x (latest version only)
- Added coverage reporting job with Codecov integration
- Added go.mod verification job
- Updated golangci-lint.yml with version specification (v2.4)
- Added permissions and configuration options
- Created new ci.yml workflow for comprehensive CI checks
- Added format checking workflow to ensure code formatting

## Breaking Changes
None - all existing workflows continue to work

## Testing
All Makefile targets tested and working:
- ✅ make help - displays organized help
- ✅ make fmt - formats code
- ✅ make lint - runs linter (with 11 existing issues)
- ✅ make test - runs tests
- ✅ make build - builds binary
- ✅ make vet - runs go vet
- ✅ make tools - installs development tools

## Notes
- golangci-lint is installed separately as a standalone binary (standard practice for Go projects)
- S1005 exclusion applied only to specific test file (values/drop_test.go) where needed
This commit introduces pre-commit hooks to enforce code quality and consistency before commits. It includes the following changes:

- **Added `.pre-commit-config.yaml`**: Defines the pre-commit hooks to be used, covering Go formatting, linting, tidying, and general checks like trailing whitespace and end-of-file enforcement.
- **Updated `Makefile`**:
    - Added `install-hooks`, `run-hooks`, and `update-hooks` targets to manage pre-commit hooks.
    - `install-hooks` ensures pre-commit is installed and sets up hooks for both pre-commit and pre-push phases.
- **Updated `CONTRIBUTING.md`**: Added instructions for setting up and using pre-commit hooks, making it clear for contributors how to maintain code quality.
- **Minor CI workflow adjustments**:
    - Cleaned up unnecessary blank lines in `.github/workflows/ci.yml`, `.github/workflows/go.yml`, and `.github/workflows/golangci-lint.yml`.
    - Removed trailing newlines in `.github/workflows/ci.yml`, `.github/workflows/go.yml`, and `.golangci.yml` to ensure clean file endings.

These changes aim to streamline the development process, catch potential issues early, and maintain a high standard of code quality.
- Convert skip-dirs/skip-files/exclude-files to v2 linters.exclusions.paths
- Convert issues.exclude-rules to linters.exclusions.rules
- Update output format to v2 structure with formats map
- Fix GitHub Actions compatibility with golangci-lint v2.4.0
- Exclude testdata directories from pre-commit hooks
- Replace golangci-lint-action with direct golangci-lint installation
- Install and run golangci-lint v2.4.0 directly, same as local development
- Avoid schema validation issues between action and config versions
- Maintain PR-specific behavior with --new-from-rev flag
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The TestAddSafeFilterNilMap function had trailing whitespace on blank
lines that was causing the Format Check CI job to fail. This commit
removes the trailing tabs/spaces from those blank lines.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@osteele osteele merged commit aed5fb6 into main Nov 6, 2025
8 checks passed
osteele pushed a commit that referenced this pull request Nov 6, 2025
- Added Unreleased section to CHANGELOG.md with recent features:
  - Jekyll Extensions Support (PR #114)
  - Auto-Escape Feature (PR #111)
  - Modernized Build Tooling (PR #115)
- Added steve-ky as contributor for ideas/feedback on PR #89

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@osteele osteele deleted the fix/lint-errors branch November 8, 2025 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants