gh-pull-request

# Creating a Pull Request for SkyWalking BanyanDB ## Branch Rules **Always create a PR from a new branch.** Never push directly to `main`. This is the same convention as the main Apache SkyWalking repo (https://github.com/apache/skywalking). ```bash git checkout -b <descriptive-branch-name> ``` ## Local Checks Before Creating PR Run these checks locally before pushing. They mirror the CI `check` job and PR-blocking tests in `.github/workflows/ci.yml`. ### Required: Code Generation and Build ```bash make generate make build ``` ### Required: Linting and Formatting ```bash make lint make check ``` `make check` verifies formatting (gofumpt), go mod tidy, and ensures no uncommitted generated file diffs. **Why this order:** `build` must succeed before `lint` so generated code exists. `check` validates consistency after linting. **After `make lint` passes:** If lint introduced any fixes (e.g. auto-formatting, field alignment corrections), commit those changes before running `make check`. Do NOT update CHANGES.md for these fixup commits — just stage all modified tracked files and commit with a message like `chore: fix lint issues`. Then run `make check` on the clean tree. ### Required: License Headers ```bash make license-check ``` All source files must have Apache 2.0 license headers. ### Required: Update CHANGES.md Add a one-line entry under the current development version section in `CHANGES.md` (at the repo root). Place it under the appropriate subsection (`### Features`, `### Bug Fixes`, etc.). ### Unit Tests Run these test packages. Each can run in parallel if the user's machine has enough cores, but it's fine to run them sequentially: ```bash make test-ci PKG=./banyand/... make test-ci PKG=./bydbctl/... make test-ci PKG=./pkg/... make test-ci PKG=./fodc/... ``` The CI uses these options: `--vv --fail-fast --label-filter \!slow` with coverage flags. For local runs, use a simplified form unless the user asks for full CI parity: ```bash TEST_CI_OPTS="--vv --fail-fast --label-filter \!slow" make test-ci PKG=./banyand/... ``` ### Integration Tests Run these after unit tests pass: ```bash make test-ci PKG=./test/integration/standalone/... make test-ci PKG=./test/integration/distributed/... ``` ## Checks NOT Practical Locally These run in CI only — no need to run locally: - **e2e tests** — require Docker + OAP stack (90 min timeout) - **fodc-e2e tests** — require Kind Kubernetes cluster - **dependency-review** — GitHub-specific action - **slow/flaky/property-repair tests** — scheduled, not PR-blocking ## Common issues - **`make lint` fails with field alignment errors**: The linter reports structs with suboptimal field ordering (e.g. `fieldalignment: struct with X pointer bytes could be Y`). Fix them automatically with: ```bash ~/go/bin/fieldalignment -fix ./path/to/package/... ``` Parse the lint output to find which packages have alignment issues, run `fieldalignment -fix` on those packages, then re-run `make lint` to confirm they're resolved. - **`make lint` fails with formatting errors**: Run `make format` to auto-fix, then re-run `make lint` to confirm. - **Test timeout**: Integration tests can be slow; add `TEST_CI_OPTS="--timeout 60m"` if they time out - **Missing tools**: `make check-req` will tell you what's missing - **`make lint` or `make check` fails with `buf: not found`**: Install `buf` automatically by running `make -C api generate`, then retry. - **`make build` fails**: Check if generated files are up to date with `make generate` ## Creating the PR ```bash git push -u origin <branch-name> gh pr create --title "<title>" --body "<body>" ``` Follow the standard PR format with a summary and test plan. ## After the PR Is Merged Merged branches must be removed, both locally and on `origin`, so they do not pile up. Do this once the PR has actually been merged — never for an open PR, and never for a PR that was closed without merging unless the user asks. Confirm the merge state first: ```bash gh pr view <number> --json state,mergedAt --jq '"\(.state) \(.mergedAt)"' ``` Only proceed when the state is `MERGED`. Then clean up: ```bash git checkout main git pull --ff-only origin main git branch -D <branch-name> git push origin --delete <branch-name> git fetch --prune origin ``` Notes: - This repo only allows **squash** merges, so the squash commit on `main` never matches the branch's own commits. `git branch -d` therefore refuses to delete even a merged branch; `-D` is required, which is why the `MERGED` check above comes first. - If `git push origin --delete` reports `remote ref does not exist`, GitHub already removed the remote branch. That is fine; continue with `git fetch --prune`. - Also prune any other local branches whose upstream is already gone (earlier merged PRs). List them, confirm with the user that each was merged, then delete: ```bash git fetch --prune origin git branch -vv | grep ': gone\]' | awk '{print $1}' ```