diff --git a/README.md b/README.md index e2976a3..00cbfd7 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,13 @@ That is because our implementation of the JSON pointer only supports explicit re the provision in the spec to resolve non-existent members as "the last element in the array", using the special trailing character "-" is not implemented. +## Other documentation + +* [All-time contributors](./CONTRIBUTORS.md) +* [Contributing guidelines](.github/CONTRIBUTING.md) +* [Maintainers documentation](docs/MAINTAINERS.md) +* [Code style](docs/STYLE.md) + [test-badge]: https://github.com/go-openapi/jsonpointer/actions/workflows/go-test.yml/badge.svg [test-url]: https://github.com/go-openapi/jsonpointer/actions/workflows/go-test.yml diff --git a/docs/MAINTAINERS.md b/docs/MAINTAINERS.md index 9f36c68..0c73976 100644 --- a/docs/MAINTAINERS.md +++ b/docs/MAINTAINERS.md @@ -1,44 +1,157 @@ # Maintainer's guide -**DRAFT** +## Repo structure -(to be completed) +Single go module. + +> **NOTE** +> +> Some `go-openapi` repos are mono-repos with multiple modules, +> with adapted CI workflows. ## Repo configuration -* branch protection -* required PR checks -* auto-merge feature +* default branch: master +* protected branches: master +* branch protection rules: + * require pull requests and approval + * required status checks: + - DCO (simple email sign-off) + - Lint + - tests completed +* auto-merge enabled (used for dependabot updates) ## Continuous Integration ### Code Quality checks * meta-linter: golangci-lint -* linter config +* linter config: [`.golangci.yml`](../.golangci.yml) (see our [posture](./STYLE.md) on linters) -* Code quality assessment: CodeFactor +* Code quality assessment: [CodeFactor](https://www.codefactor.io/dashboard) * Code quality badges - * go report card - * CodeFactor + * go report card: + * CodeFactor: + +> **NOTES** +> +> codefactor inherits roles from github. There is no need to create a dedicated account. +> +> The codefactor app is installed at the organization level (`github.com/go-openapi`). +> +> There is no special token to setup in github for CI usage. ### Testing -* test reports -* test coverage reports +* Test reports + * Uploaded to codecov: +* Test coverage reports + * Uploaded to codecov: + +* Fuzz testing + * Fuzz tests are handled separately by CI and may reuse a cached version of the fuzzing corpus. + At this moment, cache may not be shared between feature branches or feature branch and master. + The minimized corpus produced on failure is uploaded as an artifact and should be added manually + to `testdata/fuzz/...`. + +Coverage threshold status is informative and not blocking. +This is because the thresholds are difficult to tune and codecov oftentimes reports false negatives +or may fail to upload coverage. + +All tests use our fork of `stretchr/testify`: `github.com/go-openapi/testify`. +This allows for minimal test dependencies. + +> **NOTES** +> +> codecov inherits roles from github. There is no need to create a dedicated account. +> However, there is only 1 maintainer allowed to be the admin of the organization on codecov +> with their free plan. +> +> The codecov app is installed at the organization level (`github.com/go-openapi`). +> +> There is no special token to setup in github for CI usage. +> A organization-level token used to upload coverage and test reports is managed at codecov: +> no setup is required on github. ### Automated updates * dependabot + * configuration: [`dependabot.yaml`](../.github/dependabot.yaml) + + Principle: + + * codecov applies updates and security patches to the github-actions and golang ecosystems. + * all updates from "trusted" dependencies (github actions, golang.org packages, go-openapi packages + are auto-merged if they successfully pass CI. * go version udpates + Principle: + + * we support the 2 latest minor versions of the go compiler (`stable`, `oldstable`) + * `go.mod` should be updated (manually) whenever there is a new go minor release + (e.g. every 6 months). + +* contributors + * a [`CONTRIBUTORS.md`](../CONTRIBUTORS.md) file is updated weekly, with all-time contributors to the repository + * the `github-actions[bot]` posts a pull request to do that automatically + * at this moment, this pull request is not auto-approved/auto-merged (bot cannot approve its own PRs) + ### Vulnerability scanners -* github CodeQL -* trivy -* govulnscan +There are 3 complementary scanners - obviously, there is some overlap, but each has a different focus. + +* github `CodeQL` +* `trivy` +* `govulnscan` + +None of these tools require an additional account or token. + +Github CodeQL configuration is set to "Advanced", so we may collect a CI status for this check (e.g. for badges). + +Scanners run on every commit to master and at least once a week. + +Reports are centralized in github security reports for code scanning tools. ## Releases -* release notes generator: git-cliff +The release process is minimalist: + +* push a semver tag (i.e v{major}.{minor}.{patch}) to the master branch. +* the CI handles this to generate a github release with release notes + +* release notes generator: git-cliff +* configuration: [`cliff.toml`](../.cliff.toml) + +Tags are preferably PGP-signed. + +The tag message introduces the release notes (e.g. a summary of this release). + +The release notes generator does not assume that commits are necessarily "conventional commits". + +## Other files + +Standard documentation: + +* [`CONTRIBUTING.md`](../.github/CONTRIBUTING.md) guidelines +* [`DCO.md`](../.github/DCO.md) terms for first-time contributors to read +* [`CODE_OF_CONDUCT.md`](../CODE_OF_CONDUCT.md) +* [`SECURIY.md`](../SECURITY.md) policy: how to report vulnerabilities privately +* [`LICENSE`](../LICENSE) terms +* [`NOTICE`](../NOTICE) on supplementary license terms (original authors, copied code etc) + +Reference documentation (released): + +* [godoc](https://pkg.go/dev/go-openapi/jsonpointer) + +## TODOs & other ideas + +A few things remain ahead to ease a bit a maintainer's job: + +* reuse CI workflows (e.g. in `github.com/go-openapi/workflows`) +* reusable actions with custom tools pinned (e.g. in `github.com/go-openapi/gh-actions`) +* open-source license checks +* auto-merge for CONTRIBUTORS.md (requires a github app to produce tokens) +* more automated code renovation / relinting work (possibly built with CLAUDE) +* organization-level documentation web site +* ... diff --git a/docs/STYLE.md b/docs/STYLE.md index 07e352f..056fdb5 100644 --- a/docs/STYLE.md +++ b/docs/STYLE.md @@ -1,13 +1,17 @@ # Coding style at `go-openapi` -**DRAFT** - > **TL;DR** > -> We've never been super-strict on code style etc. -> But now go-openapi and go-swagger make a large codebase to maintain and keep afloat. +> Let's be honest: at `go-openapi` and `go-swagger` we've never been super-strict on code style etc. > -> Code quality and the harmonization of rules have thus become something that we need now. +> But perhaps now (2025) is the time to adopt a different stance. + +Even though our repos have been early adopters of `golangci-lint` years ago +(we used some other metalinter before), our decade-old codebase is only realigned to new rules from time to time. + +Now go-openapi and go-swagger make up a really large codebase, which is taxing to maintain and keep afloat. + +Code quality and the harmonization of rules have thus become things that we need now. ## Meta-linter @@ -17,16 +21,18 @@ You should run `golangci-lint run` before committing your changes. Many editors have plugins that do that automatically. -> We use the `golangci-lint` meta-linter. The configuration lies in `.golangci-lint.yml`. +> We use the `golangci-lint` meta-linter. The configuration lies in [`.golangci.yml`](../.golangci.yml). > You may read for additional reference. ## Linting rules posture Thanks to go's original design, we developers don't have to waste much time arguing about code figures of style. +However, the number of available linters has been growing to the point that we need to pick a choice. + We enable all linters published by `golangci-lint` by default, then disable a few ones. -Here are the reasons why they are disabled: +Here are the reasons why they are disabled (update: Nov. 2025, `golangci-lint v2.6.1`): ```yaml disable: @@ -34,26 +40,30 @@ Here are the reasons why they are disabled: - exhaustruct # we don't want to configure regexp's to check type name. That's the reviewer's job - funlen # we accept cognitive complexity as a meaningful metric, but function length is relevant - godox # we don't see any value in forbidding TODO's etc in code - - nlreturn # we usually apply this "blank line" rule to make code less compact. We just don't want to enforce it. - - nonamedreturns # we don't see any valid reason why we couldn't used named returns. + - nlreturn # we usually apply this "blank line" rule to make code less compact. We just don't want to enforce it + - nonamedreturns # we don't see any valid reason why we couldn't used named returns - noinlineerr # there is no value added forbidding inlined err - - paralleltest # we like parallel tests. We just don't want this to be enforced everywhere. + - paralleltest # we like parallel tests. We just don't want them to be enforced everywhere - recvcheck # we like the idea of having pointer and non-pointer receivers - - testpackage # we like test packages. We just don't want it to be enforced everywhere. + - testpackage # we like test packages. We just don't want them to be enforced everywhere - tparallel # see paralleltest - - varnamelen # sometimes, we like short variables + - varnamelen # sometimes, we like short variables. The linter doesn't catch cases when a short name is good - whitespace # no added value - wrapcheck # although there is some sense with this linter's general idea, it produces too much noise - - wsl # no added value. Noise. - - wsl_v5 # no added value. Noise. + - wsl # no added value. Noise + - wsl_v5 # no added value. Noise ``` -Enabled linters with relaxed constraints: +As you may see, we agree with the objective of most linters, at least the principle they are supposed to enforce. +But all linters do not support fine-grained tuning to tolerate some cases and not some others. + +When this is possible, we enable linters with relaxed constraints: ```yaml settings: dupl: threshold: 200 # in a older code base such as ours, we have to be tolerant with a little redundancy + # Hopefully, we'll be able to gradually get rid of those. goconst: min-len: 2 min-occurrences: 3 @@ -65,5 +75,9 @@ Enabled linters with relaxed constraints: default-signifies-exhaustive: true default-case-required: true lll: - line-length: 180 # we just want to avoid extremely long lines. It is no big deal if a line or two don't fit on your terminal. + line-length: 180 # we just want to avoid extremely long lines. + # It is no big deal if a line or two don't fit on your terminal. ``` + +Final note: since we have switched to a forked version of `stretchr/testify`, +we no longer benefit from the great `testifylint` linter for tests.