From 6dfe0020588ff267cd1f7cf9b4f2fe26250fd0f8 Mon Sep 17 00:00:00 2001 From: Mike Farah Date: Sun, 12 Oct 2025 14:37:05 +1100 Subject: [PATCH] Updating contrib doc --- CONTRIBUTING.md | 225 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 191 insertions(+), 34 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a6d40a8c..39b883de 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,55 +3,212 @@ Not all new PRs will be merged in It's recommended to check with the owner first (e.g. raise an issue) to discuss a new feature before developing, to ensure your hard efforts don't go to waste. -PRs to fix bugs and issues are almost always be welcome, just make sure you write tests as well. +PRs to fix bugs and issues are almost always welcome :pray: please ensure you write tests as well. -** PRs that significantly refactor code and release pipelines PRs will generally _NOT_ be accepted ** - -Significant refactors take a lot of time to understand and can have all sorts of unintended side effects. - -Release pipeline PRs are a security risk - it's too easy for a serious vulnerability to sneak in (either intended or not). If there is a new cool way of releasing things, raise an issue for discussion first - it will need to be gone over with a fine tooth comb. - -At this stage, yq is not going to maintain any other release platforms other than GitHub and Docker - that said, I'm more than happy to put in other community maintained methods in the README for visibility :heart: +The following types of PRs will _not_ be accepted: +- **Significant refactors** take a lot of time to understand and can have all sorts of unintended side effects. If you think there's a better way to do things (that requires significant changes) raise an issue for discussion first :) +- **Release pipeline PRs** are a security risk - it's too easy for a serious vulnerability to sneak in (either intended or not). If there is a new cool way of releasing things, raise an issue for discussion first - it will need to be gone over with a fine tooth comb. +- **Version bumps** are handled by dependabot, the bot will auto-raise PRs and they will be regularly merged in. +- **New release platforms** At this stage, yq is not going to maintain any other release platforms other than GitHub and Docker - that said, I'm more than happy to put in other community maintained methods in the README for visibility :heart: # Development -1. Install (Golang)[https://golang.org/] -1. Run `scripts/devtools.sh` to install the required devtools -2. Run `make [local] vendor` to install the vendor dependencies -2. Run `make [local] test` to ensure you can run the existing tests -3. Write unit tests - (see existing examples). Changes will not be accepted without corresponding unit tests. -4. Make the code changes. -5. `make [local] test` to lint code and run tests -6. Profit! ok no profit, but raise a PR and get kudos :) +## Initial Setup +1. Install [Golang](https://golang.org/) (version 1.24.0 or later) +2. Run `scripts/devtools.sh` to install required development tools: + - golangci-lint for code linting + - gosec for security analysis +3. Run `make [local] vendor` to install vendor dependencies +4. Run `make [local] test` to ensure you can run the existing tests + +## Development Workflow + +1. **Write unit tests first** - Changes will not be accepted without corresponding unit tests (see Testing section below) +2. **Make your code changes** +3. **Run tests and linting**: `make [local] test` (this runs formatting, linting, security checks, and tests) +4. **Create your PR** and get kudos! :) + +## Make Commands + +- Use `make [local] ` for local development (runs in Docker container) +- Use `make ` for CI/CD environments +- Common commands: + - `make [local] vendor` - Install dependencies + - `make [local] test` - Run all checks and tests + - `make [local] build` - Build the yq binary + - `make [local] format` - Format code + - `make [local] check` - Run linting and security checks + +# Code Quality + +## Linting and Formatting + +The project uses strict linting rules defined in `.golangci.yml`. All code must pass: + +- **Code formatting**: gofmt, goimports, gci +- **Linting**: revive, errorlint, gosec, misspell, and others +- **Security checks**: gosec security analysis +- **Spelling checks**: misspell detection + +Run `make [local] check` to verify your code meets all quality standards. + +## Code Style Guidelines + +- Follow standard Go conventions +- Use meaningful variable names +- Add comments for public functions and complex logic +- Keep functions focused and reasonably sized +- Use the project's existing patterns and conventions + +# Testing + +## Test Structure + +Tests in yq use the `expressionScenario` pattern. Each test scenario includes: +- `expression`: The yq expression to test +- `document`: Input YAML/JSON (optional) +- `expected`: Expected output +- `skipDoc`: Whether to skip documentation generation + +## Writing Tests + +1. **Find the appropriate test file** (e.g., `operator_add_test.go` for addition operations) +2. **Add your test scenario** to the `*OperatorScenarios` slice +3. **Run the specific test**: `go test -run TestAddOperatorScenarios` (replace with appropriate test name) +4. **Verify documentation generation** (see Documentation section) + +## Test Examples + +```go +var addOperatorScenarios = []expressionScenario{ + { + skipDoc: true, + expression: `"foo" + "bar"`, + expected: []string{ + "D0, P[], (!!str)::foobar\n", + }, + }, + { + document: "apples: 3", + expression: `.apples + 3`, + expected: []string{ + "D0, P[apples], (!!int)::6\n", + }, + }, +} +``` + +## Running Tests + +- **All tests**: `make [local] test` +- **Specific test**: `go test -run TestName` +- **With coverage**: `make [local] cover` # Documentation -The various operator documentation (e.g. 'strings') are generated from the 'master' branch, and have a statically defined header (e.g. `pkg/yqlib/doc/operators/headers/add.md`) and the bulk of the docs are generated from the unit tests e.g. `pkg/yqlib/operator_add_test.go`. +## Documentation Generation -The pipeline will run the tests and automatically concatenate the files together, and put them under -`pkg/qylib/doc/add.md`. These files are checked in the master branch (and are copied to the gitbook branch as part of the release process). +The project uses a documentation system that combines static headers with dynamically generated content from tests. +### How It Works -Remaining static documentation is in the 'githook' branch (where the generated docs are copied across into) +1. **Static headers** are defined in `pkg/yqlib/doc/operators/headers/*.md` +2. **Dynamic content** is generated from test scenarios in `*_test.go` files +3. **Generated docs** are created in `pkg/yqlib/doc/*.md` by concatenating headers with test-generated content +4. **Documentation is synced** to the gitbook branch for the website -## How to contribute +### Updating Operator Documentation -The first step is to find if what you want is automatically generated or not - start by looking in the master branch. +#### For Test-Generated Documentation -Note that PRs with small changes (e.g. minor typos) may not be merged (see https://joel.net/how-one-guy-ruined-hacktoberfest2020-drama). +Most operator documentation is generated from tests. To update: -### Updating dynamic documentation from master -- Search for the documentation you want to update. If you find matches in a `*_test.go` file - update that, as that will automatically update the matching `*.md` file -- Assuming you are updating a `*_test.go` file, once updated, run the test to regenerated the docs. E.g. for the 'Add' test generated docs, from the pkg/yqlib folder run: -`go test -run TestAddOperatorScenarios` which will run that test defined in the `operator_add_test.go` file. -- Ensure the tests still pass, and check the generated documentation have your update. -- Note: If the documentation is only in a `headers/*.md` file, then just update that directly -- Raise a PR to merge the changes into master! +1. **Find the test file** (e.g., `operator_add_test.go`) +2. **Update test scenarios** - each `expressionScenario` with `skipDoc: false` becomes documentation +3. **Run the test** to regenerate docs: + ```bash + cd pkg/yqlib + go test -run TestAddOperatorScenarios + ``` +4. **Verify the generated documentation** in `pkg/yqlib/doc/add.md` +5. **Create a PR** with your changes -### Updating static documentation from the gitbook branch -If you haven't found what you want to update in the master branch, then check the gitbook branch directly as there are a few pages in there that are not in master. +#### For Header-Only Documentation -- Update the `*.md` files -- Raise a PR to merge the changes into gitbook. \ No newline at end of file +If documentation exists only in `headers/*.md` files: +1. **Update the header file directly** (e.g., `pkg/yqlib/doc/operators/headers/add.md`) +2. **Create a PR** with your changes + +### Updating Static Documentation + +For documentation not in the master branch: + +1. **Check the gitbook branch** for additional pages +2. **Update the `*.md` files** directly +3. **Create a PR** to the gitbook branch + +### Documentation Best Practices + +- **Write clear, concise examples** in test scenarios +- **Use meaningful variable names** in examples +- **Include edge cases** and error conditions +- **Test your documentation changes** by running the specific test +- **Verify generated output** matches expectations + +Note: PRs with small changes (e.g. minor typos) may not be merged (see https://joel.net/how-one-guy-ruined-hacktoberfest2020-drama). + +# Troubleshooting + +## Common Setup Issues + +### Docker/Podman Issues +- **Problem**: `make` commands fail with Docker errors +- **Solution**: Ensure Docker or Podman is running and accessible +- **Alternative**: Use `make local ` to run in containers + +### Go Version Issues +- **Problem**: Build fails with Go version errors +- **Solution**: Ensure you have Go 1.24.0 or later installed +- **Check**: Run `go version` to verify + +### Vendor Dependencies +- **Problem**: `make vendor` fails or dependencies are outdated +- **Solution**: + ```bash + go mod tidy + make [local] vendor + ``` + +### Linting Failures +- **Problem**: `make check` fails with linting errors +- **Solution**: + ```bash + make [local] format # Auto-fix formatting + # Manually fix remaining linting issues + make [local] check # Verify fixes + ``` + +### Test Failures +- **Problem**: Tests fail locally but pass in CI +- **Solution**: + ```bash + make [local] test # Run in Docker container + ``` + +### Documentation Generation Issues +- **Problem**: Generated docs don't update after test changes +- **Solution**: + ```bash + cd pkg/yqlib + go test -run TestSpecificOperatorScenarios + # Check if generated file updated in pkg/yqlib/doc/ + ``` + +## Getting Help + +- **Check existing issues**: Search GitHub issues for similar problems +- **Create an issue**: If you can't find a solution, create a detailed issue +- **Ask questions**: Use GitHub Discussions for general questions +- **Join the community**: Check the project's community channels \ No newline at end of file