Skip to content

Commit a89046d

Browse files
zimnxmflendrich
authored andcommitted
Update contribution guide
Our contribution guide is very outdated and recommends style we don't use anymore. Content was updated to follow our existing style and lays out a coding convention and code review standard.
1 parent f85fd8d commit a89046d

File tree

1 file changed

+47
-113
lines changed

1 file changed

+47
-113
lines changed

docs/source/contributing.md

+47-113
Original file line numberDiff line numberDiff line change
@@ -2,154 +2,88 @@
22

33
## Prerequisites
44

5-
To develop on scylla-operator, your environment must have the following:
6-
7-
1. [Go 1.13](https://golang.org/dl/)
8-
* Make sure [GOPATH](https://github.com/golang/go/wiki/SettingGOPATH) is set to `GOPATH=$HOME/go`.
9-
2. [Kustomize v3.1.0](https://github.com/kubernetes-sigs/kustomize/releases/tag/v3.1.0)
10-
3. [kubebuilder v2.3.1](https://github.com/kubernetes-sigs/kubebuilder/releases/tag/v2.3.1)
11-
4. [Docker](https://docs.docker.com/install/)
12-
5. Git client installed
13-
6. Github account
14-
15-
To install all dependencies (Go, kustomize, kubebuilder, dep), simply run:
16-
```bash
17-
./install-dependencies.sh
18-
```
5+
To develop a scylla-operator, your environment must have the following:
6+
7+
1. [Go](https://golang.org/dl)
8+
* Check `go.mod` for minimal version requirement.
9+
2. Git client
10+
3. GitHub account
1911

2012
## Initial Setup
2113

2214
### Create a Fork
2315

24-
From your browser navigate to [http://github.com/scylladb/scylla-operator](http://github.com/scylladb/scylla-operator) and click the "Fork" button.
25-
26-
### Clone Your Fork
27-
28-
Open a console window and do the following:
29-
30-
```bash
31-
# Create the scylla operator repo path
32-
mkdir -p $GOPATH/src/github.com/scylladb
33-
34-
# Navigate to the local repo path and clone your fork
35-
cd $GOPATH/src/github.com/scylladb
36-
37-
# Clone your fork, where <user> is your GitHub account name
38-
git clone https://github.com/<user>/scylla-operator.git
39-
```
40-
41-
### Add Upstream Remote
42-
43-
First you will need to add the upstream remote to your local git:
44-
```bash
45-
# Add 'upstream' to the list of remotes
46-
git remote add upstream https://github.com/scylladb/scylla-operator.git
47-
48-
# Verify the remote was added
49-
git remote -v
50-
```
51-
Now you should have at least `origin` and `upstream` remotes. You can also add other remotes to collaborate with other contributors.
16+
Follow GitHub documentation about creating a fork: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/fork-a-repo
5217

5318
## Development
5419

5520
To add a feature or to make a bug fix, you will need to create a branch in your fork and then submit a pull request (PR) from the branch.
21+
A subset of useful commands:
22+
* `make build` - build project binaries
23+
* `make test-unit` - run unit tests
24+
* `make update` - run all code generators
25+
* `make verify` - verify if all autogenerated changes are generated.
5626

57-
### Building the project
27+
Check `make help` for more.
5828

59-
You can build the project using the Makefile commands:
60-
* Open the Makefile and change the `IMG` environment variable to a repository you have access to.
61-
* Run `make docker-push` and wait for the image to be built and uploaded in your repo.
29+
## Coding convention
6230

63-
### Create a Branch
64-
65-
From a console, create a new branch based on your fork and start working on it:
66-
67-
```bash
68-
# Ensure all your remotes are up to date with the latest
69-
git fetch --all
70-
71-
# Create a new branch that is based off upstream master. Give it a simple, but descriptive name.
72-
# Generally it will be two to three words separated by dashes and without numbers.
73-
git checkout -b feature-name upstream/master
74-
```
31+
This outlines guidelines, style suggestions, and tips for writing code throughout the Operator project.
32+
The new code should follow the rules outlined in these:
33+
* [Effective Go](https://go.dev/doc/effective_go)
34+
* [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments)
35+
* [API changes](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api_changes.md)
36+
* [API conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md)
7537

76-
Now you are ready to make the changes and commit to your branch.
7738

78-
### Updating Your Fork
39+
In case the existing code doesn't follow it, try to be consistent.
40+
Make sure to familiarize yourself with these before preparing a change.
7941

80-
During the development lifecycle, you will need to keep up-to-date with the latest upstream master. As others on the team push changes, you will need to `rebase` your commits on top of the latest. This avoids unnecessary merge commits and keeps the commit history clean.
42+
## Enhancement proposals
8143

82-
Whenever you need to update your local repository, you never want to merge. You **always** will rebase. Otherwise you will end up with merge commits in the git history. If you have any modified files, you will first have to stash them (`git stash save -u "<some description>"`).
83-
84-
```bash
85-
git fetch --all
86-
git rebase upstream/master
87-
```
88-
89-
Rebasing is a very powerful feature of Git. You need to understand how it works or else you will risk losing your work. Read about it in the [Git documentation](https://git-scm.com/docs/git-rebase), it will be well worth it. In a nutshell, rebasing does the following:
90-
- "Unwinds" your local commits. Your local commits are removed temporarily from the history.
91-
- The latest changes from upstream are added to the history
92-
- Your local commits are re-applied one by one
93-
- If there are merge conflicts, you will be prompted to fix them before continuing. Read the output closely. It will tell you how to complete the rebase.
94-
- When done rebasing, you will see all of your commits in the history.
44+
In case of bigger features or API changes, it's required to prepare an enhancement proposal beforehand as usually these require broader discussions.
45+
For further details, check https://github.com/scylladb/scylla-operator/tree/master/enhancements.
9546

9647
## Submitting a Pull Request
9748

98-
Once you have implemented the feature or bug fix in your branch, you will open a PR to the upstream repo. Before opening the PR ensure you have added unit tests, are passing the integration tests, cleaned your commit history, and have rebased on the latest upstream.
99-
100-
In order to open a pull request (PR) it is required to be up to date with the latest changes upstream. If other commits are pushed upstream before your PR is merged, you will also need to rebase again before it will be merged.
49+
Once you have implemented the feature or bug fix in your branch, you need to open a PR to the upstream repo.
50+
Before opening the PR ensure you have added unit tests, all e2e's are passing, and your commit history is clean.
10151

10252
### Commit History
10353

104-
To prepare your branch to open a PR, you will need to have the minimal number of logical commits so we can maintain
105-
a clean commit history. Most commonly a PR will include a single commit where all changes are squashed, although
106-
sometimes there will be multiple logical commits.
107-
108-
```bash
109-
# Inspect your commit history to determine if you need to squash commits
110-
git log
111-
112-
# Rebase the commits and edit, squash, or even reorder them as you determine will keep the history clean.
113-
# In this example, the last 5 commits will be opened in the git rebase tool.
114-
git rebase -i HEAD~5
115-
```
116-
117-
Once your commit history is clean, ensure you have based on the [latest upstream](#updating-your-fork) before you open the PR.
54+
To prepare your branch to open a PR, you will need to have a minimal number of logical commits so we can maintain
55+
a clean commit history. Most commonly, a PR includes a single commit where all changes are squashed and a separate commit for autogenerated changes.
11856

119-
### Commit messages
57+
### Commits and PRs
12058

121-
Please make the first line of your commit message a summary of the change that a user (not a developer) of Operator would like to read,
122-
and prefix it with the most relevant directory of the change followed by a colon.
123-
The changelog gets made by looking at just these first lines so make it good!
59+
Please make the first line of your commit message, and PR title a summary of the change that a user of Operator would like to read, written in a single sentence.
60+
Every release note is made out of just these first lines so make it good!
12461

125-
If you have more to say about the commit, then enter a blank line and carry on the description.
62+
If you have more to say about the change, then enter a blank line and carry on the description.
12663
Remember to say why the change was needed - the commit itself shows what was changed.
12764

128-
Writing more is better than less. Comparing the behaviour before the change to that after the change is very useful.
129-
Imagine you are writing to yourself in 12 months time when you've forgotten everything about what you just did, and you need to get up to speed quickly.
65+
Writing more is better than less. Comparing the behavior before the change to that after the change is beneficial.
66+
Imagine writing to yourself in 12 months when you've forgotten everything about what you just did, and you need to get up to speed quickly.
13067

131-
If the change fixes an issue then write Fixes #1234 in the commit message.
132-
This can be on the subject line if it will fit. If you don't want to close the associated issue just put #1234 and the change will get linked into the issue.
68+
If the change fixes an issue, then write Fixes #1234 in the PR description.
13369

134-
Here is an example of a short commit message:
135-
136-
```
137-
sidecar: log on reconcile loop - fixes #1234
138-
```
139-
140-
And here is an example of a longer one:
70+
Here is an example of a good commit message and PR title/description:
14171
```
14272
143-
api: now supports host networking (#1234)
73+
Add new XYZ field to ScyllaCluster CRD.
14474
145-
The operator CRD now has a "network" property that can be used to
146-
select host networking as well as setting the apropriate DNS policy.
75+
The new field allows for configuration of ZYX feature of ScyllaDB.
76+
<more details>
77+
API change was discussed in the following enhancement: <link>.
14778
14879
Fixes #1234
14980
```
15081

151-
### Submitting
82+
### Code review
83+
84+
Check [The Standard of Code Review](https://google.github.io/eng-practices/review/reviewer/standard.html) for guidelines
85+
about both doing a peer review for someone, and getting the feedback and reacting to remarks.
15286

153-
Go to the [Scylla Operator github](https://www.github.com/scylladb/scylla-operator) to open the PR. If you have pushed recently, you should see an obvious link to open the PR. If you have not pushed recently, go to the Pull Request tab and select your fork and branch for the PR.
87+
### Submitting
15488

155-
After the PR is open, you can make changes simply by pushing new commits. Your PR will track the changes in your fork and update automatically.
89+
After getting a review feedback, apply and squash the changes into initial commits, don't add new commits. Your PR should be always in the mergeable state.

0 commit comments

Comments
 (0)