Skip to content

Commit f26338a

Browse files
committed
more CONTRIBUTING etc docs
* fixes #6 Signed-off-by: Frédéric BIDON <[email protected]>
1 parent 564799a commit f26338a

File tree

6 files changed

+216
-46
lines changed

6 files changed

+216
-46
lines changed

.github/CONTRIBUTING.md

Lines changed: 138 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,124 @@
11
## Contribution Guidelines
22

3+
You'll find below general guidelines, which mostly correspond to standard practices for open sourced repositories.
4+
5+
>**TL;DR**
6+
>
7+
> If you're a seasoned go developer on github, then you should just feel at home with us and you may skip
8+
> the rest of this document. This is just the usual guideline for a go library project on github.
9+
10+
These guidelines are general to all libraries published on github by the `go-openapi` organization.
11+
12+
You'll find more detailed (or repo-specific) instructions in the [maintainer's docs](../docs).
13+
14+
### Asking questions
15+
16+
You may inquire about anything by reporting a "Question" issue on github.
17+
18+
### Reporting issues
19+
20+
Reporting a problem with our libraries _is_ a valuable contribution.
21+
22+
You can do this on the github issues page of this repository.
23+
24+
Please be as specific as possible when describing your issue.
25+
26+
Whenever relevant, please provide information about your environment (go version, OS).
27+
28+
Adding a code snippet to reproduce the issue is great, and as a big time saver for maintainers.
29+
330
### Pull requests are always welcome
431

5-
We are always thrilled to receive pull requests, and do our best to
6-
process them as fast as possible. Not sure if that typo is worth a pull
7-
request? Do it! We will appreciate it.
32+
We are always thrilled to receive pull requests, and we do our best to
33+
process them as fast as possible.
34+
35+
Not sure if that typo is worth a pull request? Do it! We will appreciate it.
36+
37+
If your pull request is not accepted on the first try, don't be discouraged!
38+
If there's a problem with the implementation, hopefully you received feedback on what to improve.
839

9-
If your pull request is not accepted on the first try, don't be
10-
discouraged! If there's a problem with the implementation, hopefully you
11-
received feedback on what to improve.
40+
We're trying very hard to keep the go-openapi packages lean and focused.
41+
These packages constitute a toolkit: it won't do everything for everybody out of the box,
42+
but everybody can use it to do just about everything related to OpenAPI.
1243

13-
We're trying very hard to keep go-swagger lean and focused. We don't want it
14-
to do everything for everybody. This means that we might decide against
15-
incorporating a new feature. However, there might be a way to implement
16-
that feature *on top of* go-swagger.
44+
This means that we might decide against incorporating a new feature.
1745

46+
However, there might be a way to implement that feature *on top of* our libraries.
47+
48+
### Environment
49+
50+
You just need a `go` compiler to be installed. No special tools are needed to work with our libraries.
51+
52+
The go compiler version required is always the old stable (latest minor go version - 1).
53+
54+
If you're already used to work with `go` you should already have everything in place.
55+
56+
Although not required, you'll be certainly more productive with a local installation of `golangci-lint`,
57+
the meta-linter our CI uses.
58+
59+
If you don't have it, you may instal it like so:
60+
61+
```sh
62+
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest
63+
```
1864

1965
### Conventions
2066

21-
Fork the repo and make changes on your fork in a feature branch:
67+
#### Git flow
68+
69+
Fork the repo and make changes to your fork in a feature branch.
70+
71+
To submit a pull request, push your branch to your fork (e.g. `upstream` remote):
72+
github will propose to open a pull request on the original repository.
73+
74+
Typically you'd follow some common naming conventions:
75+
76+
- if it's a bugfix branch, name it `fix/XXX-something`where XXX is the number of the
77+
issue on github
78+
- if it's a feature branch, create an enhancement issue to announce your
79+
intentions, and name it `feature/XXX-something` where XXX is the number of the issue.
80+
81+
> NOTE: we don't enforce naming conventions on branches: it's your fork after all.
82+
83+
#### Tests
2284

23-
- If it's a bugfix branch, name it XXX-something where XXX is the number of the
24-
issue
25-
- If it's a feature branch, create an enhancement issue to announce your
26-
intentions, and name it XXX-something where XXX is the number of the issue.
85+
Submit unit tests for your changes.
2786

28-
Submit unit tests for your changes. Go has a great test framework built in; use
29-
it! Take a look at existing tests for inspiration. Run the full test suite on
30-
your branch before submitting a pull request.
87+
Go has a great built-in test framework ; use it!
3188

32-
Update the documentation when creating or modifying features. Test
33-
your documentation changes for clarity, concision, and correctness, as
34-
well as a clean documentation build. See ``docs/README.md`` for more
35-
information on building the docs and how docs get released.
89+
Take a look at existing tests for inspiration, and run the full test suite on your branch
90+
before submitting a pull request.
3691

37-
Write clean code. Universally formatted code promotes ease of writing, reading,
38-
and maintenance. Always run `gofmt -s -w file.go` on each changed file before
39-
committing your changes. Most editors have plugins that do this automatically.
92+
#### Code style
93+
94+
You may read our stance on code style [there](../docs/STYLE.md).
95+
96+
#### Documentation
97+
98+
Update the documentation when creating or modifying features.
99+
100+
The documentation for the go-openapi packages is found on the public go docs site:
101+
102+
<https://pkg.go.dev/github.com/go-openapi/jsonpointer>
103+
104+
Test your documentation changes for clarity, concision, and correctness, as
105+
well as a clean documentation build.
106+
107+
If you want to assess the rendering of your changes when published to `pkg.go.dev`, you may
108+
want to install the `pkgsite` tool proposed by `golang.org`.
109+
110+
```sh
111+
go install golang.org/x/pkgsite/cmd/pkgsite@latest
112+
```
113+
114+
Then run on the repository folder:
115+
```sh
116+
pkgsite .
117+
```
118+
119+
This wil run a godoc server locally where you may see the documentation generated from your local repository.
120+
121+
#### Commit messages
40122

41123
Pull requests descriptions should be as clear as possible and include a
42124
reference to all the issues that they address.
@@ -47,26 +129,32 @@ Commit messages must start with a capitalized and short summary (max. 50
47129
chars) written in the imperative, followed by an optional, more detailed
48130
explanatory text which is separated from the summary by an empty line.
49131

132+
Commits that fix or close an issue should include a reference like `Closes #XXX`
133+
or `Fixes #XXX`, which will automatically close the issue when merged.
134+
135+
#### Code review
136+
50137
Code review comments may be added to your pull request. Discuss, then make the
51138
suggested modifications and push additional commits to your feature branch. Be
52139
sure to post a comment after pushing. The new commits will show up in the pull
53140
request automatically, but the reviewers will not be notified unless you
54141
comment.
55142

56-
Before the pull request is merged, make sure that you squash your commits into
57-
logical units of work using `git rebase -i` and `git push -f`. After every
58-
commit the test suite should be passing. Include documentation changes in the
59-
same commit so that a revert would remove all traces of the feature or fix.
143+
Before the pull request is merged,
144+
**make sure that you squash your commits into logical units of work**
145+
using `git rebase -i` and `git push -f`.
60146

61-
Commits that fix or close an issue should include a reference like `Closes #XXX`
62-
or `Fixes #XXX`, which will automatically close the issue when merged.
147+
After every commit the test suite should be passing.
148+
149+
Include documentation changes in the same commit so that a revert would remove all traces of the feature or fix.
63150

64151
### Sign your work
65152

66-
The sign-off is a simple line at the end of the explanation for the
67-
patch, which certifies that you wrote it or otherwise have the right to
68-
pass it on as an open-source patch. The rules are pretty simple: if you
69-
can certify the below (from
153+
The sign-off is a simple line at the end of your commit message,
154+
which certifies that you wrote it or otherwise have the right to
155+
pass it on as an open-source patch.
156+
157+
The rules are pretty simple: if you can certify the below (from
70158
[developercertificate.org](http://developercertificate.org/)):
71159

72160
```
@@ -115,3 +203,18 @@ then you just add a line to every git commit message:
115203
using your real name (sorry, no pseudonyms or anonymous contributions.)
116204

117205
You can add the sign off when creating the git commit via `git commit -s`.
206+
207+
> Notice that at this moment, we do not require commits to be PGP-signed.
208+
209+
## Issue Triage [![Open Source Helpers](https://www.codetriage.com/go-swagger/go-swagger/badges/users.svg)](https://www.codetriage.com/go-swagger/go-swagger)
210+
211+
You can help triage issues which may include:
212+
213+
* reproducing bug reports
214+
* asking for important information, such as version numbers or reproduction instructions
215+
* answering questions and sharing your insight in issue comments
216+
217+
If you would like to start triaging issues, one easy way to get started is to
218+
[subscribe to go-swagger on CodeTriage](https://www.codetriage.com/go-swagger/go-swagger).
219+
220+
> Disclaimer: the go-swagger maintainers have no affiliation with `www.codetriage.com`.

.golangci.yml

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,9 @@ linters:
33
default: all
44
disable:
55
- depguard
6-
- errorlint
7-
- exhaustruct
86
- funlen
97
- godox
10-
- gosmopolitan
11-
- ireturn
12-
- lll
8+
- exhaustruct
139
- nlreturn
1410
- nonamedreturns
1511
- noinlineerr
@@ -32,6 +28,11 @@ linters:
3228
max-complexity: 20
3329
gocyclo:
3430
min-complexity: 20
31+
exhaustive:
32+
default-signifies-exhaustive: true
33+
default-case-required: true
34+
lll:
35+
line-length: 180
3536
exclusions:
3637
generated: lax
3738
presets:
@@ -47,6 +48,7 @@ formatters:
4748
enable:
4849
- gofmt
4950
- goimports
51+
- gofumpt
5052
exclusions:
5153
generated: lax
5254
paths:

docs/MAINTAINERS.md

Whitespace-only changes.

docs/STYLE.md

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Coding style at `go-openapi`
2+
3+
> **TL;DR**
4+
>
5+
6+
## Meta-linter
7+
8+
Universally formatted go code promotes ease of writing, reading, and maintenance.
9+
10+
You should run `golangci-lint run` before committing your changes.
11+
12+
Many editors have plugins that do that automatically.
13+
14+
> We use the `golangci-lint` meta-linter. The configuration lies in `.golangci-lint.yml`.
15+
> You may read <https://golangci-lint.run/docs/linters/configuration/> for additional reference.
16+
17+
## Linting rules posture
18+
19+
Thanks to go's original design, we developers don't have to waste much time arguing about code figures of style.
20+
21+
We enable all linters published by `golangci-lint` by default, then disable a few ones.
22+
23+
Here are the reasons why they are disabled:
24+
25+
```yaml
26+
disable:
27+
- depguard # we don't want to configure rules to constrain import. That's the reviewer's job
28+
- exhaustruct # we don't want to configure regexp's to check type name. That's the reviewer's job
29+
- funlen # we accept cognitive complexity as a meaningful metric, but function length is relevant
30+
- godox # we don't see any value in forbidding TODO's etc in code
31+
- nlreturn
32+
- nonamedreturns
33+
- noinlineerr # there is no value added forbidding inlined err
34+
- paralleltest # we like parallel tests. We just don't want this to be enforced everywhere.
35+
- recvcheck # we like the idea of having pointer and non-pointer receivers
36+
- testpackage # we like test packages. We just don't want it to be enforced everywhere.
37+
- tparallel # see paralleltest
38+
- varnamelen # sometimes, we like short variables
39+
- whitespace # no added value
40+
- wrapcheck # although there is some sense with this linter's general idea, it produces too much noise
41+
- wsl # no added value. Noise.
42+
- wsl_v5 # no added value. Noise.
43+
```
44+
45+
Enabled linters with relaxed constraints:
46+
```yaml
47+
settings:
48+
dupl:
49+
threshold: 200 # in a older code base such as ours, we have to be tolerant with a little redundancy
50+
goconst:
51+
min-len: 2
52+
min-occurrences: 3
53+
cyclop:
54+
max-complexity: 20 # the default is too low for most of our functions. 20 is a nicer trade-off
55+
gocyclo:
56+
min-complexity: 20
57+
exhaustive: # when using default in switch, this should be good enough
58+
default-signifies-exhaustive: true
59+
default-case-required: true
60+
lll:
61+
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.
62+
```

pointer.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ type JSONSetable interface {
5454
// # Limitations
5555
//
5656
// - Unlike go standard marshaling, untagged fields do not default to the go field name and are ignored.
57-
// - anonymous embedded fields are not
57+
// - anonymous embedded fields are not traversed
5858
type Pointer struct {
5959
referenceTokens []string
6060
}
@@ -251,7 +251,7 @@ func (p *Pointer) resolveNodeForToken(node any, decodedToken string, nameProvide
251251
rValue := reflect.Indirect(reflect.ValueOf(node))
252252
kind := rValue.Kind()
253253

254-
switch kind { //nolint:exhaustive
254+
switch kind {
255255
case reflect.Struct:
256256
nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken)
257257
if !ok {
@@ -294,7 +294,7 @@ func isNil(input any) bool {
294294
}
295295

296296
kind := reflect.TypeOf(input).Kind()
297-
switch kind { //nolint:exhaustive
297+
switch kind {
298298
case reflect.Pointer, reflect.Map, reflect.Slice, reflect.Chan:
299299
return reflect.ValueOf(input).IsNil()
300300
default:
@@ -338,7 +338,7 @@ func getSingleImpl(node any, decodedToken string, nameProvider *jsonname.NamePro
338338
return getSingleImpl(*typed, decodedToken, nameProvider)
339339
}
340340

341-
switch kind { //nolint:exhaustive
341+
switch kind {
342342
case reflect.Struct:
343343
nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken)
344344
if !ok {
@@ -389,7 +389,7 @@ func setSingleImpl(node, data any, decodedToken string, nameProvider *jsonname.N
389389
return ns.JSONSet(decodedToken, data)
390390
}
391391

392-
switch rValue.Kind() { //nolint:exhaustive
392+
switch rValue.Kind() {
393393
case reflect.Struct:
394394
nm, ok := nameProvider.GetGoNameForType(rValue.Type(), decodedToken)
395395
if !ok {
@@ -462,7 +462,7 @@ func offsetSingleObject(dec *json.Decoder, decodedToken string) (int64, error) {
462462
func offsetSingleArray(dec *json.Decoder, decodedToken string) (int64, error) {
463463
idx, err := strconv.Atoi(decodedToken)
464464
if err != nil {
465-
return 0, fmt.Errorf("token reference %q is not a number: %v: %w", decodedToken, err, ErrPointer)
465+
return 0, fmt.Errorf("token reference %q is not a number: %w: %w", decodedToken, err, ErrPointer)
466466
}
467467
var i int
468468
for i = 0; i < idx && dec.More(); i++ {

pointer_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ func (s settableDoc) MarshalJSON() ([]byte, error) {
471471
res.D = s.Int
472472
return json.Marshal(res)
473473
}
474+
474475
func (s *settableDoc) UnmarshalJSON(data []byte) error {
475476
var res struct {
476477
A settableColl `json:"a"`
@@ -550,6 +551,7 @@ type settableColl struct {
550551
func (s settableColl) MarshalJSON() ([]byte, error) {
551552
return json.Marshal(s.Items)
552553
}
554+
553555
func (s *settableColl) UnmarshalJSON(data []byte) error {
554556
return json.Unmarshal(data, &s.Items)
555557
}
@@ -583,6 +585,7 @@ type settableInt struct {
583585
func (s settableInt) MarshalJSON() ([]byte, error) {
584586
return json.Marshal(s.Value)
585587
}
588+
586589
func (s *settableInt) UnmarshalJSON(data []byte) error {
587590
return json.Unmarshal(data, &s.Value)
588591
}

0 commit comments

Comments
 (0)