CASSJAVA-30: Update CONTRIBUTING.md by SiyaoIsHiding · Pull Request #2041 · apache/cassandra-java-driver

SiyaoIsHiding · 2025-05-19T20:18:57Z

No description provided.

…ng-md

lukasz-antoniak · 2025-10-16T10:30:36Z

CONTRIBUTING.md

-but in no way discouraged: it's generally a good idea to have a class-level comment that explains
-where the component fits in the architecture, and anything else that you feel is important.
+- **API packages:** must be documented.
+- **Internal packages:** optional but encouraged—add a short class-level comment explaining the component’s role.


encouraged - add

lukasz-antoniak · 2025-10-16T10:33:12Z

CONTRIBUTING.md

+Testing against an external process, using either of the tools:
+- [Simulacron](https://github.com/datastax/simulacron): simulates Cassandra nodes on loopback. You “prime” it with expected query results.  
+  *Example*: `NodeTargetingIT`.
+- [CCM](https://github.com/pcmanus/ccm): launches actual Cassandra nodes locally. The `ccm` executable must be in the path. Pass variables like `-Dccm.version=5.0.0 -Dccm.dse=false` to specify the Cassandra version or distribution.


Should be -ccm.distribution=dse, not -Dccm.dse=false.

lukasz-antoniak · 2025-10-16T10:39:02Z

CONTRIBUTING.md

+1. Nullability: Please use the [Spotbugs annotations](https://spotbugs.github.io) to document nullability of new class or interface using the `@Nullable` and `@NonNull` annotations from the package `edu.umd.cs.findbugs.annotations`.
+2. Concurrency annotations: Please use the [JCIP annotations](http://jcip.net/annotations/doc/index.html) to document thread-safety.
+3. Static imports: Please only use static imports for AssertJ, Mockito, and Awaitility.
+4. `toString()`: only for debug in development. Use `toCqlLiteral()` is applicable for logs.


Shorten format does not tell the meaning of previous long description. Not all classes would have toCqlLiteral().

lukasz-antoniak · 2025-10-16T10:41:05Z

CONTRIBUTING.md

+#### Integration Tests
+
+1. Install Cassandra Cluster Manager (CCM) following its [README](https://github.com/apache/cassandra-ccm).
+2. On macOS, enable loopback aliases:


Maybe explicitly mention On MacOS only, ...

absurdfarce · 2025-10-29T18:30:32Z

I feel like the current state of this PR loses a lot of good suggestions and guidance about how new code should be structured. I'm wondering if we can't make a smaller set of edits to update what needs to be changed while still keeping most of the guidance in the current doc.

SiyaoIsHiding · 2025-10-30T01:37:51Z

I think realistically, no community contributor is gonna read 500+ lines before they submit a single PR. It makes sense for the onboarding of a full-time job, but it does not make sense for a community contributor, e.g., someone who maintains a library that uses the Java driver as a dependency and only plans to create one pull request. When they open such a long file, even if the file is nice and detailed, they are just gonna skip it.

If you agree ^ this is true, then we gotta make it shorter.

My 2 cents is that the following has lower priority and can be potentially removed:

Examples to make a point stronger. E.g.

/**
* @return this {@link Builder} <-- completely unnecessary
*/
Builder withLimit(int limit) {

Things that users can easily find out by themselves. E.g.

We use SLF4J; loggers are declared like this:
private static final Logger LOG = LoggerFactory.getLogger(TheEnclosingClass.class);

Things that are very general or common sense, nothing specific to this Java driver project. E.g.

Like commits, pull requests should be focused on a single, clearly stated goal..
If you have to address feedback, avoid rewriting the history (e.g. squashing or amending commits): this makes the reviewers' job harder, because they have to re-read the full diff and figure out where your new changes are.

Tips on a scenario that is unlikely to happen, and when it happens, we can easily fix it in the PR review. E.g.

Don't abuse the stream API
The java.util.stream API is often used (abused?) as a "functional API for collections"

It's rare that a community contributor submits a PR with this functional stream API. And when it happens, we can fix it in the PR review. Such things should be of low priority.

I think those points ^ are all good to have, but are less important than a short CONTRIBUTING.md so that people actually read.

But if you both think the longer version is better, I will rewrite it. Pls let me know what you think :)

SiyaoIsHiding · 2025-11-26T01:20:16Z

To remind myself: add instruction on how to change the stack size, either in this CONTRIBUTING.md, or CCM, or both.

SiyaoIsHiding · 2025-12-10T06:10:55Z

I added the existing CONTRIBUTING.md and made it long again. Please review at your earliest convenience!

SiyaoIsHiding · 2025-12-16T18:53:32Z

To remind myself, add JNA workaround for Mac M1:

mvn dependency:get -Dartifact=net.java.dev.jna:jna:5.10.0
cp ~/.m2/repository/net/java/dev/jna/jna/5.10.0/jna-5.10.0.jar ~/.ccm/repository/4.0.19/lib/jna-5.6.0.jar

absurdfarce

A few things I really think need to be changed here, specifically the removal of the Google Group and the specification of a minimum Maven version. Otherwise my comments are fairly minor.

absurdfarce · 2026-03-04T21:14:48Z

CONTRIBUTING.md


-### XML
+### Communication
+1. **Mailing List**: https://groups.google.com/a/lists.datastax.com/g/java-driver-user


This Google Group is no longer a means of communication for this project. The Python and CPP drivers both point users at ASF Slack instead (see here as an example) and we should as well.

It looks like the top-level README also needs to get a similar update... that's outside the scope of this PR and can be handled separately.

absurdfarce · 2026-03-04T21:16:45Z

CONTRIBUTING.md

+
+All code changes require:
+
+1. **A corresponding JIRA ticket**  


We've been asked this question a few times and in general our policy is a bit loose here. We don't require a JIRA ticket for changes and I'm not sure I want to give the impression that that's a roadblock for users contributing code. We should certainly say that users are encouraged to create a JIRA ticket describing the problem and perhaps including what they'd like to see done to fix it... and then follow that up with a PR.

absurdfarce · 2026-03-04T21:17:45Z

CONTRIBUTING.md

+
+**Do not** base a PR on another one.
+
+**Do not** squash commits before the PR is ready to merge.


Out of curiosity... why not? If a user squashes commits on their PR branch and pushes what's the harm done?

For example, if there is a PR that has one commit for a massive rename, and subsequent commits doing other things, then we want to keep that commit history in case we want to revert the massive rename.
But this is one of the many things in this CONTRIBUTING.md that I find trivial.

agreed, makes it harder to understand what has changed if the history changes during a review.

I guess I don't see this as a huge problem but agree it doesn't hurt anything to prohibit it in the general case.

absurdfarce · 2026-03-04T21:22:38Z

CONTRIBUTING.md

+### Prerequisites
+
+- **Java 8+** 
+- **Maven**


We have a minimum Maven version that's supported for the project; probably need to explicitly include that here. CASSJAVA-102 bumped that minimum up to 3.8.1 at least for 4.x.

absurdfarce · 2026-03-04T21:24:17Z

CONTRIBUTING.md

+
+## Development Setup
+
+### Prerequisites


This raises an interesting point: the pre-reqs mentioned below apply to 4.x but are different for 3.x. I don't know that we need to enumerate a full set of separate requirements for 3.x distinct from those for 4.x but we probably should at least say (1) these reqs apply to 4.x and (2) 3.x may have different (and likely older) requirements.

If they want a development set up for 3.x then they should just look for the CONTRIBUTING.md in the 3.x branch. https://github.com/apache/cassandra-java-driver/blob/3.x/CONTRIBUTING.md

Ah, entirely fair point... we can ignore this one!

absurdfarce · 2026-03-04T21:27:51Z

CONTRIBUTING.md

+     ```
+     mvn clean install -DskipTests
+     ```
+   - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`.


IIRC this is a two-step process:

The user has to install a local version of the shaded Guava JAR into their Maven cache (which is what the command above does)

The user has to ignore the project in the "Maven" flyout window in the upper right (because we want them to fall back into the version that's now installed in their Maven cache).

From my experience, I didn't need to ignore the IntelliJ built-in maven, I just had to make the IntelliJ built-in maven to install the cache, too.

Hmmmm, that was very definitely not adequate for me... but I'm fine rolling with this for now. We can deal with it on a case-by-case basis via Slack and update this if it becomes a problem (which I doubt it will).

absurdfarce · 2026-03-04T21:32:41Z

CONTRIBUTING.md

+#### Integration Tests
+
+1. Install Cassandra Cluster Manager (CCM) following its [README](https://github.com/apache/cassandra-ccm).
+2. On MacOS only, enable loopback aliases:


This is added here for... Simulacron testing, right? Might be worth calling that out explicitly. As it stands now this text appears to refer to something required for ccm testing which I don't think is correct.

Isn't loopback aliases for CCM testing? I always need to turn on loopback aliases whenever I use CCM

yeah its needed for simulacron too since it uses the same local IP addresses. You don't have to install anything though, i guess we can note it here:

Suggested change

2. On MacOS only, enable loopback aliases:

2. **MacOS only**, for CCM and Simulacron-based tests, enable loopback aliases:

I don't have personal knowledge of the requirement; I don't run any of this on OS X. I'm just going by what the original CONTRIBUTING doc said on the topic.

SiyaoIsHiding · 2026-03-05T00:54:08Z

PR updated!

tolbertam · 2026-03-23T13:58:36Z

CONTRIBUTING.md

+  - [General](#general)
+  - [Code Formatting and License Headers](#code-formatting-and-license-headers)
+  - [Javadoc](#javadoc)
+  - [Logging](#logging)


Looks like the actual section is titled Logs so this link is broken, but I like Logging better so mind changing the section name at line 231?

👍 I very much agree that "Logging" is a better description for that section.

CONTRIBUTING.md

tolbertam · 2026-03-23T14:17:52Z

CONTRIBUTING.md

+```bash
+mvn clean install -DskipTests


Was curious about this, is running:

mvn clean install -DskipTests

before

mvn test

Actually necessary now?

I just purged my maven repo (rm -rf ~/.m2/repository) and ran mvn test as is and it worked.

looks like this was required on #2045, but i suspect if on more recent commit this isn't needed anymore.

if i rebase 2045 everything works fine

This used to be required because of the need to have the shaded Guava JAR in place when compiling tests IIRC... I think I've still seen that behaviour fairly recently.

tolbertam · 2026-03-23T14:25:40Z

CONTRIBUTING.md

+```shell
+for i in {2..255}; do sudo ifconfig lo0 alias 127.0.0.$i up; done
+   ```
+   Note: This may slow down networking. To remove the aliases after testing:


Unfortunately this looks to have gotten even worse on newer mac os versions. I wonder if we can get away with creating less aliases...will see if I can find a minimum.

no need to change this for now, will find the minimum and can address separately

tolbertam · 2026-03-23T14:42:25Z

CONTRIBUTING.md

+   ```
+   To target a specific Cassandra version or distribution:
+   ```
+   mvn verify -Dccm.version=3.11.0


We should bump this to a newer version, primarily because a version this old won't work on arm based Macs, and also its nice to test against latest (more tests will run)

Suggested change

mvn verify -Dccm.version=3.11.0

mvn verify -Dccm.version=5.0.6

However, I think for this to work we need a pointer to JDK 11, so maybe for now we target 4.1.11, and can think about how to update docs around running tests against newer cassandra versions as well which require newer JDKs:

Suggested change

mvn verify -Dccm.version=3.11.0

mvn verify -Dccm.version=4.1.10

After some testing , I think we can just use 5.0.6 with JDK 11 just fine, we should just advise using Java 11 in the docs (see above), i think it will be the most dependable and also has the advantage of being able to run most of the tests.

Lets keep this @ 3.11.0 for now, as running with JDK 11 doesn't work reliably, will address in follow on

It's meant as an example here for the user; presumably they'd tweak it for their own use case when they actually run it. There's certainly no harm in bumping this up to a newer version but even using 3.11.0 seems to illustrate the point well enough.

tolbertam · 2026-03-23T14:45:01Z

CONTRIBUTING.md

-## Pre-commit hook (highly recommended)
-
-Ensure `pre-commit.sh` is executable, then run:
-
-```
-ln -s ../../pre-commit.sh .git/hooks/pre-commit
-```
-
-This will only allow commits if the tests pass. It is also a good reminder to keep the test suite
-short.
-
-Note: the tests run on the current state of the working directory. I tried to add a `git stash` in
-the script to only test what's actually being committed, but I couldn't get it to run reliably
-(it's still in there but commented). Keep this in mind when you commit, and don't forget to re-add
-the changes if the first attempt failed and you fixed the tests.
-
-## Speeding up the build for local tests
-
-If you need to install something in your local repository quickly, you can use the `fast` profile to
-skip all "non-essential" checks (licenses, formatting, tests, etc):
-
-```
-mvn clean install -Pfast
-```
-
-You can speed things up even more by targeting specific modules with the `-pl` option:
-
-```
-mvn clean install -Pfast -pl core,query-builder,mapper-runtime,mapper-processor,bom
-```
-
-Please run the normal build at least once before you push your changes.


Noticed this section was removed, but we did update pre-commit.sh should we keep this doc?

tolbertam · 2026-03-23T14:46:40Z

CONTRIBUTING.md

-## Commits
-
-Keep your changes **focused**. Each commit should have a single, clear purpose expressed in its 
-message.
-
-Resist the urge to "fix" cosmetic issues (add/remove blank lines, move methods, etc.) in existing
-code. This adds cognitive load for reviewers, who have to figure out which changes are relevant to
-the actual issue. If you see legitimate issues, like typos, address them in a separate commit (it's
-fine to group multiple typo fixes in a single commit).
-
-Isolate trivial refactorings into separate commits. For example, a method rename that affects dozens
-of call sites can be reviewed in a few seconds, but if it's part of a larger diff it gets mixed up
-with more complex changes (that might affect the same lines), and reviewers have to check every
-line.
-
-Commit message subjects start with a capital letter, use the imperative form and do **not** end
-with a period:
-
-* correct: "Add test for CQL request handler"
-* incorrect: "~~Added test for CQL request handler~~"
-* incorrect: "~~New test for CQL request handler~~"
-
-Avoid catch-all messages like "Minor cleanup", "Various fixes", etc. They don't provide any useful
-information to reviewers, and might be a sign that your commit contains unrelated changes.
-
-We don't enforce a particular subject line length limit, but try to keep it short.
-
-You can add more details after the subject line, separated by a blank line. The following pattern
-(inspired by [Netty](http://netty.io/wiki/writing-a-commit-message.html)) is not mandatory, but
-welcome for complex changes:
-
-```
-One line description of your change
-
-Motivation:
-
-Explain here the context, and why you're making that change.
-What is the problem you're trying to solve.
-
-Modifications:
-
-Describe the modifications you've done.
-
-Result:
-
-After your change, what will change.


I think this was generally good advice, but not something that has really been enforced for a while, so +1 to removing.

tolbertam · 2026-03-23T15:14:10Z

CONTRIBUTING.md

+
+### Prerequisites
+
+- **Java 8+** 


Something that i bumped into is that it is very difficult to run with an up to date JDK 8 as newer OpenJDK build fail on Cassandra setting a very low stack size:

ccm start --wait-for-binary-proto --config-dir=/var/folders/ts/m0zplxmx30dcyp3ffzkgwpyw0000gn/T/ccm8054909345564425947 -v b'\nThe stack size specified is too small, Specify at least 640k\n'

I wonder if for this reason, and that newer Cassandra versions (5.0+) require JDK11 we just advise folks use Java 11 at point?

Suggested change

- **Java 8+**

- **Java 8+** (Java 11+ is recommended)

Actually, lets keep this at Java 8 for now as I had some issues running with JDK 11, i'll create a follow on JIRA to address this 👍

CONTRIBUTING.md

tolbertam

This looks great, thank you @SiyaoIsHiding ! Added some commentary but this looks great. ready to +1 with some small adjustments

Co-authored-by: Andrew Tolbert <6889771+tolbertam@users.noreply.github.com>

absurdfarce

I think I'm largely okay with the state of this change now; whatever nits I have are small enough that they can be addressed at some future date (if they need to be addressed at all). And there is so much goodness in this PR that I'd rather get it in than hold it up on nits.

I'm going to hold on giving an explicit 👍 until @tolbertam confirms that he's okay with what's here as well; I want to make sure all of his comments/questions are addressed to his satisfaction.

absurdfarce · 2026-03-25T05:22:58Z

CONTRIBUTING.md

+  - [General](#general)
+  - [Code Formatting and License Headers](#code-formatting-and-license-headers)
+  - [Javadoc](#javadoc)
+  - [Logging](#logging)


👍 I very much agree that "Logging" is a better description for that section.

absurdfarce · 2026-03-25T07:07:20Z

CONTRIBUTING.md

+
+## Development Setup
+
+### Prerequisites


Ah, entirely fair point... we can ignore this one!

absurdfarce · 2026-03-25T07:08:33Z

CONTRIBUTING.md

+     ```
+     mvn clean install -DskipTests
+     ```
+   - If IntelliJ uses a different Maven version, use the Maven window in IntelliJ: under `Lifecycle`, click `clean` and then `install`.


Hmmmm, that was very definitely not adequate for me... but I'm fine rolling with this for now. We can deal with it on a case-by-case basis via Slack and update this if it becomes a problem (which I doubt it will).

absurdfarce · 2026-03-25T07:09:41Z

CONTRIBUTING.md

+```bash
+mvn clean install -DskipTests


This used to be required because of the need to have the shaded Guava JAR in place when compiling tests IIRC... I think I've still seen that behaviour fairly recently.

absurdfarce · 2026-03-25T07:12:24Z

CONTRIBUTING.md

+#### Integration Tests
+
+1. Install Cassandra Cluster Manager (CCM) following its [README](https://github.com/apache/cassandra-ccm).
+2. On MacOS only, enable loopback aliases:


I don't have personal knowledge of the requirement; I don't run any of this on OS X. I'm just going by what the original CONTRIBUTING doc said on the topic.

absurdfarce · 2026-03-25T07:13:48Z

CONTRIBUTING.md

+   ```
+   To target a specific Cassandra version or distribution:
+   ```
+   mvn verify -Dccm.version=3.11.0


It's meant as an example here for the user; presumably they'd tweak it for their own use case when they actually run it. There's certainly no harm in bumping this up to a newer version but even using 3.11.0 seems to illustrate the point well enough.

absurdfarce · 2026-03-25T07:16:11Z

CONTRIBUTING.md

+
+**Do not** base a PR on another one.
+
+**Do not** squash commits before the PR is ready to merge.


I guess I don't see this as a huge problem but agree it doesn't hurt anything to prohibit it in the general case.

absurdfarce · 2026-03-25T07:18:11Z

CONTRIBUTING.md

-this makes the reviewers' job harder, because they have to re-read the full diff and figure out
-where your new changes are. Instead, push a new commit on top of the existing history; it will be
-squashed later when the PR gets merged. If the history is complex, it's a good idea to indicate in
-the message where the changes should be squashed:


Interesting that we used to have this instruction in the CONTRIBUTING doc. It obviously doesn't apply anymore now that we've moved away from squash merges... but it does suggest to me that this might be a good place to provide instructions to a user about how do a squash via an interactive rebase. We can do that for user-contributed PRs but I'd kind of prefer to put that on the contributors if we can.

SiyaoIsHiding added 9 commits May 5, 2025 19:36

java 8

adbdd24

WIP

9adec6c

loopback

5c1caa5

fmt

692c64b

hook

ab1233e

hook

4a24e97

CONTRIBUTING.md and pre commit hook

afc1d38

reset cqlrequesthandler

efc2fd7

Merge branch '4.x' of github.com:datastax/java-driver into contributi…

2529ff3

…ng-md

absurdfarce mentioned this pull request Sep 15, 2025

CASSJAVA-109: Manual and API References after donation #2055

Merged

SiyaoIsHiding added 2 commits October 15, 2025 13:04

add coding guide

fca6351

git stash back

b00e40e

lukasz-antoniak reviewed Oct 16, 2025

View reviewed changes

add braces

22c8a4b

SiyaoIsHiding added 5 commits December 9, 2025 21:43

make it long again

d091ae6

add -Dccm.distribution

2d55aba

revert formatting that IntelliJ did

462e7e3

more formatting

06f58ee

add communication

ff00fff

tolbertam self-requested a review February 23, 2026 18:54

aratno approved these changes Feb 23, 2026

View reviewed changes

absurdfarce requested changes Mar 4, 2026

View reviewed changes

Address PR review

55c7cf0

tolbertam reviewed Mar 23, 2026

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

tolbertam reviewed Mar 23, 2026

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

tolbertam reviewed Mar 23, 2026

View reviewed changes

SiyaoIsHiding and others added 2 commits March 23, 2026 10:07

Update CONTRIBUTING.md

7702821

Co-authored-by: Andrew Tolbert <6889771+tolbertam@users.noreply.github.com>

Update CONTRIBUTING.md

5f38fa6

Co-authored-by: Andrew Tolbert <6889771+tolbertam@users.noreply.github.com>

absurdfarce reviewed Mar 25, 2026

View reviewed changes


		Do not base a PR on another one.

		Do not squash commits before the PR is ready to merge.

	2. On MacOS only, enable loopback aliases:
	2. MacOS only, for CCM and Simulacron-based tests, enable loopback aliases:

	mvn verify -Dccm.version=3.11.0
	mvn verify -Dccm.version=5.0.6

	mvn verify -Dccm.version=3.11.0
	mvn verify -Dccm.version=4.1.10

Conversation

SiyaoIsHiding commented May 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukasz-antoniak Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukasz-antoniak Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

absurdfarce commented Oct 29, 2025

Uh oh!

SiyaoIsHiding commented Oct 30, 2025

Uh oh!

SiyaoIsHiding commented Nov 26, 2025

Uh oh!

SiyaoIsHiding commented Dec 10, 2025

Uh oh!

SiyaoIsHiding commented Dec 16, 2025

Uh oh!

absurdfarce left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tolbertam Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SiyaoIsHiding commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lukasz-antoniak Oct 16, 2025 •

edited

Loading

lukasz-antoniak Oct 16, 2025 •

edited

Loading

tolbertam Mar 23, 2026 •

edited

Loading

tolbertam Mar 23, 2026 •

edited

Loading

absurdfarce left a comment •

edited

Loading