Skip to content

CASSJAVA-30: Update CONTRIBUTING.md#2041

Open
SiyaoIsHiding wants to merge 17 commits intoapache:4.xfrom
SiyaoIsHiding:contributing-md
Open

CASSJAVA-30: Update CONTRIBUTING.md#2041
SiyaoIsHiding wants to merge 17 commits intoapache:4.xfrom
SiyaoIsHiding:contributing-md

Conversation

@SiyaoIsHiding
Copy link
Contributor

No description provided.

CONTRIBUTING.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

encouraged - add

CONTRIBUTING.md Outdated
Copy link
Member

@lukasz-antoniak lukasz-antoniak Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

CONTRIBUTING.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

CONTRIBUTING.md Outdated
Copy link
Member

@lukasz-antoniak lukasz-antoniak Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explicitly mention On MacOS only, ...

@absurdfarce
Copy link
Contributor

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
Copy link
Contributor Author

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:

  1. Examples to make a point stronger. E.g.
/**
* @return this {@link Builder} <-- completely unnecessary
*/
Builder withLimit(int limit) {
  1. 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);

  1. 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.

  1. 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
Copy link
Contributor Author

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

@SiyaoIsHiding
Copy link
Contributor Author

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

@SiyaoIsHiding
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments