Skip to content

Commit be76d48

Browse files
committed
docs: added maintainer's doc and style guide
Signed-off-by: Frederic BIDON <[email protected]>
1 parent c22925c commit be76d48

File tree

3 files changed

+165
-31
lines changed

3 files changed

+165
-31
lines changed

README.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,13 @@ That is because our implementation of the JSON pointer only supports explicit re
9797
the provision in the spec to resolve non-existent members as "the last element in the array",
9898
using the special trailing character "-" is not implemented.
9999

100+
## Other documentation
101+
102+
* [All-time contributors](./CONTRIBUTORS.md)
103+
* [Contributing guidelines](.github/CONTRIBUTING.md)
104+
* [Maintainers documentation](docs/MAINTAINERS.md)
105+
* [Code style](docs/STYLE.md)
106+
100107
<!-- Badges: status -->
101108
[test-badge]: https://github.com/go-openapi/jsonpointer/actions/workflows/go-test.yml/badge.svg
102109
[test-url]: https://github.com/go-openapi/jsonpointer/actions/workflows/go-test.yml

docs/MAINTAINERS.md

Lines changed: 128 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,157 @@
11
# Maintainer's guide
22

3-
**DRAFT**
3+
## Repo structure
44

5-
(to be completed)
5+
Single go module.
6+
7+
> **NOTE**
8+
>
9+
> Some `go-openapi` repos are mono-repos with multiple modules,
10+
> with adapted CI workflows.
611
712
## Repo configuration
813

9-
* branch protection
10-
* required PR checks
11-
* auto-merge feature
14+
* default branch: master
15+
* protected branches: master
16+
* branch protection rules:
17+
* require pull requests and approval
18+
* required status checks:
19+
- DCO (simple email sign-off)
20+
- Lint
21+
- tests completed
22+
* auto-merge enabled (used for dependabot updates)
1223

1324
## Continuous Integration
1425

1526
### Code Quality checks
1627

1728
* meta-linter: golangci-lint
18-
* linter config
29+
* linter config: [`.golangci.yml`](../.golangci.yml) (see our [posture](./STYLE.md) on linters)
1930

20-
* Code quality assessment: CodeFactor
31+
* Code quality assessment: [CodeFactor](https://www.codefactor.io/dashboard)
2132
* Code quality badges
22-
* go report card
23-
* CodeFactor
33+
* go report card: <https://goreportcard.com/>
34+
* CodeFactor: <https://goreportcard.com/>
35+
36+
> **NOTES**
37+
>
38+
> codefactor inherits roles from github. There is no need to create a dedicated account.
39+
>
40+
> The codefactor app is installed at the organization level (`github.com/go-openapi`).
41+
>
42+
> There is no special token to setup in github for CI usage.
2443
2544
### Testing
2645

27-
* test reports
28-
* test coverage reports
46+
* Test reports
47+
* Uploaded to codecov: <https://app.codecov.io/analytics/gh/go-openapi>
48+
* Test coverage reports
49+
* Uploaded to codecov: <https://app.codecov.io/gh/go-openapi>
50+
51+
* Fuzz testing
52+
* Fuzz tests are handled separately by CI and may reuse a cached version of the fuzzing corpus.
53+
At this moment, cache may not be shared between feature branches or feature branch and master.
54+
The minimized corpus produced on failure is uploaded as an artifact and should be added manually
55+
to `testdata/fuzz/...`.
56+
57+
Coverage threshold status is informative and not blocking.
58+
This is because the thresholds are difficult to tune and codecov oftentimes reports false negatives
59+
or may fail to upload coverage.
60+
61+
All tests use our fork of `stretchr/testify`: `github.com/go-openapi/testify`.
62+
This allows for minimal test dependencies.
63+
64+
> **NOTES**
65+
>
66+
> codecov inherits roles from github. There is no need to create a dedicated account.
67+
> However, there is only 1 maintainer allowed to be the admin of the organization on codecov
68+
> with their free plan.
69+
>
70+
> The codecov app is installed at the organization level (`github.com/go-openapi`).
71+
>
72+
> There is no special token to setup in github for CI usage.
73+
> A organization-level token used to upload coverage and test reports is managed at codecov:
74+
> no setup is required on github.
2975
3076
### Automated updates
3177

3278
* dependabot
79+
* configuration: [`dependabot.yaml`](../.github/dependabot.yaml)
80+
81+
Principle:
82+
83+
* codecov applies updates and security patches to the github-actions and golang ecosystems.
84+
* all updates from "trusted" dependencies (github actions, golang.org packages, go-openapi packages
85+
are auto-merged if they successfully pass CI.
3386

3487
* go version udpates
3588

89+
Principle:
90+
91+
* we support the 2 latest minor versions of the go compiler (`stable`, `oldstable`)
92+
* `go.mod` should be updated (manually) whenever there is a new go minor release
93+
(e.g. every 6 months).
94+
95+
* contributors
96+
* a [`CONTRIBUTORS.md`](../CONTRIBUTORS.md) file is updated weekly, with all-time contributors to the repository
97+
* the `github-actions[bot]` posts a pull request to do that automatically
98+
* at this moment, this pull request is not auto-approved/auto-merged (bot cannot approve its own PRs)
99+
36100
### Vulnerability scanners
37101

38-
* github CodeQL
39-
* trivy
40-
* govulnscan
102+
There are 3 complementary scanners - obviously, there is some overlap, but each has a different focus.
103+
104+
* github `CodeQL`
105+
* `trivy` <https://trivy.dev/docs/latest/getting-started>
106+
* `govulnscan` <https://go.dev/blog/govulncheck>
107+
108+
None of these tools require an additional account or token.
109+
110+
Github CodeQL configuration is set to "Advanced", so we may collect a CI status for this check (e.g. for badges).
111+
112+
Scanners run on every commit to master and at least once a week.
113+
114+
Reports are centralized in github security reports for code scanning tools.
41115

42116
## Releases
43117

44-
* release notes generator: git-cliff
118+
The release process is minimalist:
119+
120+
* push a semver tag (i.e v{major}.{minor}.{patch}) to the master branch.
121+
* the CI handles this to generate a github release with release notes
122+
123+
* release notes generator: git-cliff <https://git-cliff.org/docs/>
124+
* configuration: [`cliff.toml`](../.cliff.toml)
125+
126+
Tags are preferably PGP-signed.
127+
128+
The tag message introduces the release notes (e.g. a summary of this release).
129+
130+
The release notes generator does not assume that commits are necessarily "conventional commits".
131+
132+
## Other files
133+
134+
Standard documentation:
135+
136+
* [`CONTRIBUTING.md`](../.github/CONTRIBUTING.md) guidelines
137+
* [`DCO.md`](../.github/DCO.md) terms for first-time contributors to read
138+
* [`CODE_OF_CONDUCT.md`](../CODE_OF_CONDUCT.md)
139+
* [`SECURIY.md`](../SECURITY.md) policy: how to report vulnerabilities privately
140+
* [`LICENSE`](../LICENSE) terms
141+
* [`NOTICE`](../NOTICE) on supplementary license terms (original authors, copied code etc)
142+
143+
Reference documentation (released):
144+
145+
* [godoc](https://pkg.go/dev/go-openapi/jsonpointer)
146+
147+
## TODOs & other ideas
148+
149+
A few things remain ahead to ease a bit a maintainer's job:
150+
151+
* reuse CI workflows (e.g. in `github.com/go-openapi/workflows`)
152+
* reusable actions with custom tools pinned (e.g. in `github.com/go-openapi/gh-actions`)
153+
* open-source license checks
154+
* auto-merge for CONTRIBUTORS.md (requires a github app to produce tokens)
155+
* more automated code renovation / relinting work (possibly built with CLAUDE)
156+
* organization-level documentation web site
157+
* ...

docs/STYLE.md

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
# Coding style at `go-openapi`
22

3-
**DRAFT**
4-
53
> **TL;DR**
64
>
7-
> We've never been super-strict on code style etc.
8-
> But now go-openapi and go-swagger make a large codebase to maintain and keep afloat.
5+
> Let's be honest: at `go-openapi` and `go-swagger` we've never been super-strict on code style etc.
96
>
10-
> Code quality and the harmonization of rules have thus become something that we need now.
7+
> But perhaps now (2025) is the time to adopt a different stance.
8+
9+
Even though our repos have been early adopters of `golangci-lint` years ago
10+
(we used some other metalinter before), our decade-old codebase is only realigned to new rules from time to time.
11+
12+
Now go-openapi and go-swagger make up a really large codebase, which is taxing to maintain and keep afloat.
13+
14+
Code quality and the harmonization of rules have thus become things that we need now.
1115

1216
## Meta-linter
1317

@@ -17,43 +21,49 @@ You should run `golangci-lint run` before committing your changes.
1721

1822
Many editors have plugins that do that automatically.
1923

20-
> We use the `golangci-lint` meta-linter. The configuration lies in `.golangci-lint.yml`.
24+
> We use the `golangci-lint` meta-linter. The configuration lies in [`.golangci.yml`](../.golangci.yml).
2125
> You may read <https://golangci-lint.run/docs/linters/configuration/> for additional reference.
2226
2327
## Linting rules posture
2428

2529
Thanks to go's original design, we developers don't have to waste much time arguing about code figures of style.
2630

31+
However, the number of available linters has been growing to the point that we need to pick a choice.
32+
2733
We enable all linters published by `golangci-lint` by default, then disable a few ones.
2834

29-
Here are the reasons why they are disabled:
35+
Here are the reasons why they are disabled (update: Nov. 2025, `golangci-lint v2.6.1`):
3036

3137
```yaml
3238
disable:
3339
- depguard # we don't want to configure rules to constrain import. That's the reviewer's job
3440
- exhaustruct # we don't want to configure regexp's to check type name. That's the reviewer's job
3541
- funlen # we accept cognitive complexity as a meaningful metric, but function length is relevant
3642
- godox # we don't see any value in forbidding TODO's etc in code
37-
- nlreturn # we usually apply this "blank line" rule to make code less compact. We just don't want to enforce it.
38-
- nonamedreturns # we don't see any valid reason why we couldn't used named returns.
43+
- nlreturn # we usually apply this "blank line" rule to make code less compact. We just don't want to enforce it
44+
- nonamedreturns # we don't see any valid reason why we couldn't used named returns
3945
- noinlineerr # there is no value added forbidding inlined err
40-
- paralleltest # we like parallel tests. We just don't want this to be enforced everywhere.
46+
- paralleltest # we like parallel tests. We just don't want them to be enforced everywhere
4147
- recvcheck # we like the idea of having pointer and non-pointer receivers
42-
- testpackage # we like test packages. We just don't want it to be enforced everywhere.
48+
- testpackage # we like test packages. We just don't want them to be enforced everywhere
4349
- tparallel # see paralleltest
44-
- varnamelen # sometimes, we like short variables
50+
- varnamelen # sometimes, we like short variables. The linter doesn't catch cases when a short name is good
4551
- whitespace # no added value
4652
- wrapcheck # although there is some sense with this linter's general idea, it produces too much noise
47-
- wsl # no added value. Noise.
48-
- wsl_v5 # no added value. Noise.
53+
- wsl # no added value. Noise
54+
- wsl_v5 # no added value. Noise
4955
```
5056
51-
Enabled linters with relaxed constraints:
57+
As you may see, we agree with the objective of most linters, at least the principle they are supposed to enforce.
58+
But all linters do not support fine-grained tuning to tolerate some cases and not some others.
59+
60+
When this is possible, we enable linters with relaxed constraints:
5261
5362
```yaml
5463
settings:
5564
dupl:
5665
threshold: 200 # in a older code base such as ours, we have to be tolerant with a little redundancy
66+
# Hopefully, we'll be able to gradually get rid of those.
5767
goconst:
5868
min-len: 2
5969
min-occurrences: 3
@@ -65,5 +75,9 @@ Enabled linters with relaxed constraints:
6575
default-signifies-exhaustive: true
6676
default-case-required: true
6777
lll:
68-
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.
78+
line-length: 180 # we just want to avoid extremely long lines.
79+
# It is no big deal if a line or two don't fit on your terminal.
6980
```
81+
82+
Final note: since we have switched to a forked version of `stretchr/testify`,
83+
we no longer benefit from the great `testifylint` linter for tests.

0 commit comments

Comments
 (0)