Skip to content

Commit 38cc48e

Browse files
adambkaplanadambkaplan
committed
Improve contributor guidelines
* Move existing contributing guide to docs/HACKING.md * Created new CONTRIBUTING.md focusing on submitting issues and PRs.
1 parent 13e74a7 commit 38cc48e

File tree

2 files changed

+198
-71
lines changed

2 files changed

+198
-71
lines changed

‎CONTRIBUTING.md

+85-71
Original file line numberDiff line numberDiff line change
@@ -1,113 +1,127 @@
1-
Hacking on source-to-image
2-
==========================
1+
# Contributing Guide
32

4-
## Local development
3+
## Filing Bugs
54

6-
S2I comes with a `Makefile` which defines following targets:
5+
Bug reports may be filed as an issue on GitHub, with the label `kind/bug`.
6+
The label can be added by placing the [Prow](https://prow.svc.ci.openshift.org/command-help?repo=openshift%2Fsource-to-image)
7+
command `/kind bug` in your issue.
78

8-
* `build` - is the default target responsible for building S2I binary, under the covers
9-
it calls `hack/build-go.sh`. The resulting binary will be placed in `_output/local/go/bin/`.
10-
* `all` - is synonym for `build`.
11-
* `test` - is responsible for testing S2I, under the covers it calls `hack/test-go.sh`.
12-
Additionally you can pass `WHAT` or `TEST` variable specifying directory names to test,
13-
eg. `make test WHAT=pkg/build`
14-
* `check` - is synonym for `test`.
15-
* `clean` - cleans environment by removing `_output`
9+
A well-written bug report has the following format:
1610

17-
## Generating Bash completion
11+
```markdown
12+
**Is this a feature request or bug?**
1813

19-
If you are modifying or adding sub-command or command flag, make sure you update
20-
the generated Bash completion and include it in your PR. To update Bash
21-
completion, you can run the following command:
14+
/kind bug
2215

23-
$ hack/update-generated-completions.sh
16+
**What went wrong?**
2417

25-
This will regenerate the `./contrib/bash/s2i` file.
18+
Enter a description of what went wrong.
2619

27-
## Test Suites
20+
**Steps to reproduce:**
2821

29-
S2I uses two levels of testing - unit tests and integration tests, both of them are run on each
30-
pull request, so make sure to run those before submitting one.
22+
1. Step 1
23+
2. Step 2
24+
3. Step 3
3125

26+
**Expected results:**
3227

33-
### Unit tests
28+
Enter what you expected to happen.
3429

35-
Unit tests follow standard Go conventions and are intended to test the behavior and output of a
36-
single package in isolation. All code is expected to be easily testable with mock interfaces and
37-
stubs, and when they are not it usually means that there's a missing interface or abstraction in the
38-
code. A unit test should focus on testing that branches and error conditions are properly returned
39-
and that the interface and code flows work as described. Unit tests can depend on other packages but
40-
should not depend on other components.
30+
**Actual results:**
4131

42-
The unit tests for an entire package should not take more than 0.5s to run, and if they do, are
43-
probably not really unit tests or need to be rewritten to avoid sleeps or pauses. Coverage on a unit
44-
test should be above 70% unless the units are a special case.
32+
Enter what actually occurred, including command line output
4533

46-
Run the unit tests with:
34+
**Version:**
4735

48-
$ hack/test-go.sh
36+
s2i: Enter output of `s2i version`
37+
docker: Enter the output of `docker version` if you are using docker to build container images.
4938

50-
or an individual package unit test with:
39+
**Additional info:**
5140

52-
$ hack/test-go.sh pkg/build/strategies/sti
41+
Add any relevant context here.
42+
```
5343

54-
To run only a certain regex of tests in a package, use:
44+
## Submitting Feature Requests
5545

56-
$ hack/test-go.sh pkg/build/strategies/sti -test.run=TestLayeredBuild
46+
Feature requests are likewise submitted as GitHub issues, with the `kind/feature` label.
47+
The label can be added by placing the [Prow](https://prow.svc.ci.openshift.org/command-help?repo=openshift%2Fsource-to-image)
48+
command `/kind feature` in your issue.
5749

58-
To get verbose output add `-v` to the end:
50+
A well-written feature request has the following format:
5951

60-
$ hack/test-go.sh pkg/build/strategies/sti -test.run=TestLayeredBuild -v
52+
```markdown
53+
**Is this a feature request or bug?**
6154

62-
To run all tests with verbose output:
55+
/kind feature
6356

64-
$ hack/test-go.sh "" -v
57+
**User Stories**
6558

66-
To run tests without the Go race detector, which is on by default, use:
59+
As a developer using source-to-image
60+
I would like ...
61+
So that ...
6762

68-
$ S2I_RACE="" hack/test-go.sh
63+
(you may add more than one story to provide further use cases to consider)
6964

70-
A line coverage report is printed by default. To turn it off, use:
65+
**Additional info:**
7166

72-
$ S2I_COVER="" hack/test-go.sh
67+
Add any relevant context or deeper description here.
68+
```
7369

74-
To create an HTML coverage report for all packages:
70+
## Submitting a Pull Request
7571

76-
$ OUTPUT_COVERAGE=/tmp/s2i-cover hack/test-go.sh
72+
You can contribute to source-to-image by submitting a pull request ("PR").
73+
Any member of the OpenShift organization can review your PR and signal their approval with the
74+
`/lgtm` command. Only approvers listed in the [OWNERS](OWNERS) file may add the `approved` label to
75+
your PR. In order for PR to merge, it must have the following:
7776

77+
1. The `approved` label
78+
2. The `lgtm` label
79+
3. All tests passing in CI, which is managed by Prow.
7880

79-
### Integration tests
81+
If you are not a member of the OpenShift GitHub organization, an OpenShift team member will need to
82+
add the `ok-to-test` label to your PR for our CI tests to run.
8083

81-
The second category are integration tests which verify the whole S2I flow. The integration tests
82-
require a couple of images for testing, these can be built with `hack/build-test-images.sh`, if
83-
integration tests don't find them it'll print appropriate information regarding running this command.
84+
### Feature or Bugfix PRs
8485

85-
Run the integration tests with:
86+
Pull requests which implement a feature or fix a bug should consist of a single commit with the
87+
full code changes. Commit messages should have the following structure:
8688

87-
$ hack/test-integration.sh
89+
```text
90+
<Title - under 50 characters>
8891
92+
<Body - under 100 characters per line>
8993
90-
## Dependency Management
94+
<Footer>
95+
```
9196

92-
S2I uses [Go Modules](https://github.com/golang/go/wiki/Modules) for `vendor` directory
93-
management. Please consider [`go.mod`](./go.mod) to see how dependencies are organized in this
94-
project, and consider official documentation about
95-
[Go Modules](https://github.com/golang/go/wiki/Modules#how-to-use-modules).
97+
The title is required, and should be under 50 characters long. This is a short description of your
98+
change.
9699

97-
The basic usage of `go mod` in this project is:
100+
The body is optional, and may contain a longer description of the changes in the commit. Lines
101+
should not exceed 100 characters in length for readability.
98102

99-
$ go mod tidy -v
100-
$ go mod vendor
101-
$ go mod verify
103+
The footer is optional, and may contain information such as sign-off lines. If your code is related
104+
to a GitHub issue, add it here with the text `Fixes xxx`.
102105

103-
## Building a Release
106+
### Work In Progress PRs
104107

105-
To build a S2I release you run `make release` on a system with Docker,
106-
which will create a build environment image and then execute a cross platform Go build within it. The build
107-
output will be copied to `_output/releases` as a set of tars containing each version.
108+
Contributors who would like feedback but are still making code modifications can create "work in
109+
progress" PRs by adding "WIP" to the PR title. Once work is done and ready for final review, please
110+
remove "WIP" from your PR title and [squash your commits](https://medium.com/@slamflipstrom/a-beginners-guide-to-squashing-commits-with-git-rebase-8185cf6e62ec).
108111

109-
1. Create a new git tag `git tag vX.X.X -a -m "vX.X.X" HEAD`
110-
2. Push the tag to GitHub `git push origin --tags` where `origin` is `github.com/openshift/source-to-image.git`
111-
4. Run `make release`
112-
5. Upload the binary artifacts generated by that build to GitHub release page.
113-
6. Send an email to the dev list, including the important changes prior to the release.
112+
### Updating dependencies
113+
114+
Pull requests which update dependencies are an exception to the "one commit" rule.
115+
To better track dependency changes, PRs with dependency updates should have the following structure:
116+
117+
1. A commit with changes to `go.mod` and `go.sum` declaring the new or updated dependencies.
118+
2. A commit with updates to vendored code, with `bump(<modules>)` in the title:
119+
1. If only a small number of modules are updated, list them in the parentheses.
120+
Example: `bump(containers/image/v5):`
121+
2. If several modules are updated, use `bump(*)` as the title.
122+
3. Include a reason for updating dependencies in the commit body.
123+
3. If necessary, add a commit with reactions to vendored code changes (ex - method signature
124+
updates).
125+
4. If necessary, add a commit with code that takes advantage of the newly vendored code.
126+
127+
See [HACKING.md](docs/HACKING.md#dependency-management) for more information on how to update dependencies.

‎docs/HACKING.md

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
Hacking on source-to-image
2+
==========================
3+
4+
## Local development
5+
6+
S2I comes with a `Makefile` which defines following targets:
7+
8+
* `build` - is the default target responsible for building S2I binary, under the covers
9+
it calls `hack/build-go.sh`. The resulting binary will be placed in `_output/local/go/bin/`.
10+
* `all` - is synonym for `build`.
11+
* `test` - is responsible for testing S2I, under the covers it calls `hack/test-go.sh`.
12+
Additionally you can pass `WHAT` or `TEST` variable specifying directory names to test,
13+
eg. `make test WHAT=pkg/build`
14+
* `check` - is synonym for `test`.
15+
* `clean` - cleans environment by removing `_output`
16+
17+
## Generating Bash completion
18+
19+
If you are modifying or adding sub-command or command flag, make sure you update
20+
the generated Bash completion and include it in your PR. To update Bash
21+
completion, you can run the following command:
22+
23+
$ hack/update-generated-completions.sh
24+
25+
This will regenerate the `./contrib/bash/s2i` file.
26+
27+
## Test Suites
28+
29+
S2I uses two levels of testing - unit tests and integration tests, both of them are run on each
30+
pull request, so make sure to run those before submitting one.
31+
32+
33+
### Unit tests
34+
35+
Unit tests follow standard Go conventions and are intended to test the behavior and output of a
36+
single package in isolation. All code is expected to be easily testable with mock interfaces and
37+
stubs, and when they are not it usually means that there's a missing interface or abstraction in the
38+
code. A unit test should focus on testing that branches and error conditions are properly returned
39+
and that the interface and code flows work as described. Unit tests can depend on other packages but
40+
should not depend on other components.
41+
42+
The unit tests for an entire package should not take more than 0.5s to run, and if they do, are
43+
probably not really unit tests or need to be rewritten to avoid sleeps or pauses. Coverage on a unit
44+
test should be above 70% unless the units are a special case.
45+
46+
Run the unit tests with:
47+
48+
$ hack/test-go.sh
49+
50+
or an individual package unit test with:
51+
52+
$ hack/test-go.sh pkg/build/strategies/sti
53+
54+
To run only a certain regex of tests in a package, use:
55+
56+
$ hack/test-go.sh pkg/build/strategies/sti -test.run=TestLayeredBuild
57+
58+
To get verbose output add `-v` to the end:
59+
60+
$ hack/test-go.sh pkg/build/strategies/sti -test.run=TestLayeredBuild -v
61+
62+
To run all tests with verbose output:
63+
64+
$ hack/test-go.sh "" -v
65+
66+
To run tests without the Go race detector, which is on by default, use:
67+
68+
$ S2I_RACE="" hack/test-go.sh
69+
70+
A line coverage report is printed by default. To turn it off, use:
71+
72+
$ S2I_COVER="" hack/test-go.sh
73+
74+
To create an HTML coverage report for all packages:
75+
76+
$ OUTPUT_COVERAGE=/tmp/s2i-cover hack/test-go.sh
77+
78+
79+
### Integration tests
80+
81+
The second category are integration tests which verify the whole S2I flow. The integration tests
82+
require a couple of images for testing, these can be built with `hack/build-test-images.sh`, if
83+
integration tests don't find them it'll print appropriate information regarding running this command.
84+
85+
Run the integration tests with:
86+
87+
$ hack/test-integration.sh
88+
89+
90+
## Dependency Management
91+
92+
S2I uses [Go Modules](https://github.com/golang/go/wiki/Modules) for `vendor` directory
93+
management. Please consider [`go.mod`](./go.mod) to see how dependencies are organized in this
94+
project, and consider official documentation about
95+
[Go Modules](https://github.com/golang/go/wiki/Modules#how-to-use-modules).
96+
97+
The basic usage of `go mod` in this project is:
98+
99+
$ go mod tidy -v
100+
$ go mod vendor
101+
$ go mod verify
102+
103+
## Building a Release
104+
105+
To build a S2I release you run `make release` on a system with Docker,
106+
which will create a build environment image and then execute a cross platform Go build within it. The build
107+
output will be copied to `_output/releases` as a set of tars containing each version.
108+
109+
1. Create a new git tag `git tag vX.X.X -a -m "vX.X.X" HEAD`
110+
2. Push the tag to GitHub `git push origin --tags` where `origin` is `github.com/openshift/source-to-image.git`
111+
4. Run `make release`
112+
5. Upload the binary artifacts generated by that build to GitHub release page.
113+
6. Send an email to the dev list, including the important changes prior to the release.

0 commit comments

Comments
 (0)