Skip to content

Commit 48ea5d4

Browse files
committed
Update CONTRIBUTING.md
The section for the commit message format is too generic, so this commit adds more details and a clear example of how a commit message should be structured. This commit also makes it clear in CONTRIBUTING.md that JIRA should be used instead of GH issues and encourages contributors to seek feedback and discussion on the proposed changes early. patch by João Reis; reviewed by TBD for CASSGO-10
1 parent 253be52 commit 48ea5d4

File tree

1 file changed

+83
-12
lines changed

1 file changed

+83
-12
lines changed

CONTRIBUTING.md

Lines changed: 83 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,34 @@
11
# Contributing to the Apache Cassandra GoCQL Driver
22

3-
**TL;DR** - this manifesto sets out the bare minimum requirements for submitting a patch to gocql.
3+
**TL;DR** This manifesto sets out the bare minimum requirements for submitting a patch to gocql. It also offers some suggestions to speed up the review, approve and merge process.
44

55
This guide outlines the process of landing patches in gocql and the general approach to maintaining the code base.
66

77
## Background
88

9-
The goal of the gocql project is to provide a stable and robust CQL driver for Go. This is a community driven project that is coordinated by a small team of developers in and around the Apache Cassandra project. For security, governance and administration issues please refer to the Cassandra Project Management Committee.
9+
The goal of the gocql project is to provide a stable and robust CQL driver for Go. This is a community driven project that is coordinated by a small team of developers in and around the Apache Cassandra project. For security, governance and administration issues please refer to the Cassandra Project Management Committee.
10+
11+
## Engage with the community early
12+
13+
If you are interested in contributing a particular feature or bug fix we heavily encourage you to start a discussion with the community or at the very least announce your interest in working on it before jumping right into writing code. It helps reduce the likelihood of multiple people working on the same issue in parallel when they could be collaborating instead. Getting feedback early in the contribution process will also greatly speed up the review, approval and merge process.
14+
15+
Common ways to engage with the GoCQL community are:
16+
17+
- CASSGO project on ASF JIRA
18+
- Apache Cassandra dev mailing list
19+
- #cassandra-drivers channel on ASF Slack
1020

1121
## Minimum Requirement Checklist
1222

1323
The following is a check list of requirements that need to be satisfied in order for us to merge your patch:
1424

15-
* You should raise a pull request to apache/cassandra-gocql-driver on Github
16-
* The pull request has a title that clearly summarizes the purpose of the patch
25+
* A JIRA issue exists in the [CASSGO Project](https://issues.apache.org/jira/projects/CASSGO/issues/) for the proposed changes
26+
* If the proposed changes are significant then ideally a discussion about the implementation approach should happen before a PR is even opened (prototyping is fine though)
27+
* Pull request raised to apache/cassandra-gocql-driver on Github
28+
* The pull request has a title that clearly summarizes the purpose of the patch and references the relevant CASSGO JIRA issue if there is one
1729
* The motivation behind the patch is clearly defined in the pull request summary
30+
* CHANGES.txt is updated with the JIRA issue if it is clear which release the PR is targeting and there is a JIRA for it already (otherwise the committer that merges the PR will take care of this)
31+
* This is not required if the change is not "releasable" (e.g. changes to documentation, CI, etc.)
1832
* You agree that your contribution is donated to the Apache Software Foundation (appropriate copyright is on all new files)
1933
* The patch will merge cleanly
2034
* The test coverage does not fall
@@ -23,21 +37,72 @@ The following is a check list of requirements that need to be satisfied in order
2337
* Notable changes (i.e. new features or changed behavior, bugfixes) are appropriately documented in CHANGELOG.md, functional changes also in godoc
2438
* A correctly formatted commit message, see below
2539

26-
If there are any requirements that can't be reasonably satisfied, please state this either on the pull request or as part of discussion on the mailing list. Where appropriate, the core team may apply discretion and make an exception to these requirements.
40+
If there are any requirements that can't be reasonably satisfied, please state this either on the pull request or as part of discussion on the mailing list, JIRA or slack. Where appropriate, the core team may apply discretion and make an exception to these requirements.
2741

2842
## Commit Message
2943

30-
The Apache Cassandra project has a commit message precendence like
44+
The commit message format should be:
45+
3146
```
32-
<One sentence description, usually Jira title or CHANGES.txt summary>
47+
<short description>
48+
49+
<reason why the change is needed>
3350
34-
patch by <Authors>; reviewed by <Reviewers> for CASSANDRA-#####
51+
Patch by <authors>; reviewed by <Reviewers> for CASSGO-#####
3552
```
3653

37-
The 'patch by …; reviewed by' line is important. It permits our review-than-commit procedure, allowing commits from non-git-branch patches. It is also parsed to build the project contribulyse statistics found [here](https://nightlies.apache.org/cassandra/devbranch/misc/contribulyze/html/).
54+
Short description should:
55+
* Be a short sentence.
56+
* Start with a capital letter.
57+
* Be written in the present tense.
58+
* Summarize what is changed, not why it is changed.
3859

60+
Short description should not:
61+
* End with a period.
62+
* Use the word Fixes . Most commits fix something.
63+
64+
Long description / Reason:
65+
* Should describe why the change is needed. What is fixed by the change? Why it it was broken before? What use case does the new feature solve?
66+
* Consider adding details of other options that you considered when implementing the change and why you made the design decisions you made.
67+
68+
The `patch by …; reviewed by` line is important. It is parsed to build the project contribulyse statistics found [here](https://nightlies.apache.org/cassandra/devbranch/misc/contribulyze/html/).
69+
70+
Some tips from the Apache Cassandra Project's "How to Commit" documentation: https://cassandra.apache.org/_/development/how_to_commit.html#tips
71+
72+
#### Example commit message:
73+
74+
```
75+
Increase default timeouts to 11s
3976
40-
Background: https://cassandra.apache.org/_/development/how_to_commit.html#tips
77+
Client timeouts need to be higher than server timeouts,
78+
so that work does not accumulate on the server with retries.
79+
If the client timeout is shorter than a server timeout,
80+
the client can start a retry while the original request
81+
is still running on the server.
82+
83+
The default gocql default timeout was lower
84+
than the Cassandra default timeout.
85+
86+
Cassandra has multiple server timeout options,
87+
most of them are less or equal to 10s by default as of Cassandra 4.1:
88+
89+
read_request_timeout 5s
90+
range_request_timeout 10s
91+
write_request_timeout 2s
92+
counter_write_request_timeout 5s
93+
cas_contention_timeout 1s
94+
truncate_request_timeout 60s
95+
request_timeout 10s
96+
97+
Truncate is an uncommon operation, so we can use 11s as the default
98+
timeout.
99+
100+
patch by John Doe, Jane Doe; reviewed by Bob Smith, Jane Smith for CASSGO-#####
101+
```
102+
103+
### Signing commits
104+
105+
Signing commits with a pgp or ssh key is heavily encouraged although not required.
41106

42107
## Beyond The Checklist
43108

@@ -55,8 +120,14 @@ That said, the point of writing tests is to provide a safety net to catch regres
55120

56121
### Sign Off Procedure
57122

58-
Generally speaking, a pull request can get merged by any one of the project's committers. If your change is minor, chances are that one team member will just go ahead and merge it there and then. As stated earlier, suitable test coverage will increase the likelihood that a single reviewer will assess and merge your change. If your change has no test coverage, or looks like it may have wider implications for the health and stability of the library, the reviewer may elect to refer the change to another team member to achieve consensus before proceeding. Therefore, the tighter and cleaner your patch is, the quicker it will go through the review process.
123+
A Pull Request needs +1s from two committers before it can be merged (or one +1 if the author is a committer).
124+
125+
As stated earlier, suitable test coverage will increase the likelihood that a PR will be approved and merged. If your change has no test coverage, or looks like it may have wider implications for the health and stability of the library, the reviewers may elect to refer the change to other members of the community to achieve consensus before proceeding. Therefore, the tighter and cleaner your patch is, the quicker it will go through the review process.
59126

60127
### Supported Features
61128

62-
gocql is a low level wire driver for Cassandra CQL. By and large, we would like to keep the functional scope of the library as narrow as possible. We think that gocql should be tight and focused, and we will be naturally skeptical of things that could just as easily be implemented in a higher layer. Inevitably you will come across something that could be implemented in a higher layer, save for a minor change to the core API. In this instance, please strike up a conversation in the Cassandra community. Chances are we will understand what you are trying to achieve and will try to accommodate this in a maintainable way.
129+
gocql is a low level wire driver for Cassandra CQL. By and large, we would like to keep the functional scope of the library as narrow as possible. We think that gocql should be tight and focused, and we will be naturally skeptical of things that could just as easily be implemented in a higher layer.
130+
131+
Inevitably you will come across something that could be implemented in a higher layer, save for a minor change to the core API. In this instance, please strike up a conversation in the Cassandra community.
132+
133+
Chances are we will understand what you are trying to achieve and will try to accommodate this in a maintainable way.

0 commit comments

Comments
 (0)