Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copyright to ASF #1682

Closed

Conversation

michaelsembwever
Copy link
Member

@michaelsembwever michaelsembwever marked this pull request as draft July 13, 2023 13:50
@michaelsembwever michaelsembwever force-pushed the mck/asf-donation/4.x branch 2 times, most recently from 8c220ba to e705036 Compare September 28, 2023 12:10
@michaelsembwever
Copy link
Member Author

michaelsembwever commented Sep 28, 2023

Example checks

  • rg -i "Copyright DataStax" (manual replacement of license header)
  • rg -il "DataStax Java driver" | xargs sed -i '' 's/DataStax Java driver/Java Driver/gI'

Patches for other branches:

@hhughes
Copy link
Contributor

hhughes commented Sep 28, 2023

git show -- ':!*.java'

If you want to look at any changes which aren't to .java files

@hhughes
Copy link
Contributor

hhughes commented Sep 28, 2023

I notice in the new license we're using Apache Cassandra - no retail mark or anything else.

There are a few places where we use various different symbles after the name if we should be tidying those up at the same time:

% grep -h -r -o -E "Apache Cassandra[^ $]+" . | sort | uniq -c
   2 Apache Cassandra©
  10 Apache Cassandra®
   1 Apache Cassandra™.
  27 Apache Cassandra(R)
   4 Apache Cassandra(R)'s
   1 Apache Cassandra(R).
   1 Apache Cassandra(R).</description>
   3 Apache Cassandra,
   7 Apache Cassandra®
   1 Apache Cassandra®.
   1 Apache Cassandra®:
   1 Apache Cassandra®]
   1 Apache Cassandra®]:
  13 Apache CassandraⓇ

@hhughes
Copy link
Contributor

hhughes commented Sep 28, 2023

Github review UI is unusable for me with this patch :) I'm going to add feeback as main comments

diff --git a/manual/core/non_blocking/README.md b/manual/core/non_blocking/README.md
index a40779f8e..123d9ef1a 100644
--- a/manual/core/non_blocking/README.md
+++ b/manual/core/non_blocking/README.md
@@ -11,7 +11,7 @@ such as [Vert.x] or [Reactor], along with tools for automatic detection of block
 [Reactor]: https://projectreactor.io
 [BlockHound]: https://github.com/reactor/BlockHound
 
-**In summary, when used properly, the DataStax Java driver offers non-blocking guarantees for most 
+**In summary, when used properly, the Java driver offers non-blocking guarantees for most 
 of its operations, and during most of the session lifecycle.**
 
 These guarantees and their exceptions are detailed below. A final chapter explains how to use the 

Nit 1: you've added a trailing space to the end of this changed line. sorry this is normal, not sure why git highlighted it such an angry red for me
Nit 2: Do we want to take this opportunity settle on casing of Java Driver vs Java driver? (most of those lines have been modified in this change)

@hhughes
Copy link
Contributor

hhughes commented Sep 28, 2023

There are still a few matches for otherly cased DataStax Java Driver:

% grep -rl "DataStax Java Driver" --exclude-dir=.git 
./docs.yaml
./core/src/main/java/com/datastax/dse/driver/api/core/cql/continuous/ContinuousSession.java
./README.md
./manual/cloud/README.md
./examples/pom.xml
./examples/README.md
./osgi-tests/README.md

@michaelsembwever
Copy link
Member Author

Apache Cassandra

should be Apache Cassandra® on the first usage on any page. but this is not critical. (can be addressed later on.)

There are still a few matches for otherly cased DataStax Java Driver

on it…
Will standardise to "Java Driver" too.

@michaelsembwever
Copy link
Member Author

All branches done.

@michaelsembwever michaelsembwever marked this pull request as ready for review September 29, 2023 12:47
@hhughes
Copy link
Contributor

hhughes commented Sep 29, 2023

diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationFactor.java b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationFactor.java
index f20a472d9..966372da6 100644
--- a/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationFactor.java
+++ b/core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationFactor.java
@@ -1,12 +1,14 @@
 package com.datastax.oss.driver.internal.core.metadata.token;
 /*
- * Copyright DataStax, Inc.
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
...

Nit: the license in core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationFactor.java comes after the package declaration (this is how it was before your refactoring) - any concern?

@michaelsembwever
Copy link
Member Author

Nit: the license in core/src/main/java/com/datastax/oss/driver/internal/core/metadata/token/ReplicationFactor.java comes after the package declaration (this is how it was before your refactoring) - any concern?

no concern

@hhughes
Copy link
Contributor

hhughes commented Sep 29, 2023

% grep -rin "DataStax driver" --exclude-dir=.git 
./mapper-processor/src/main/java/com/datastax/oss/driver/internal/mapper/processor/SingleFileCodeGenerator.java:28:      "Generated by the DataStax driver mapper, do not edit directly.\n";

Nit: one mention of DataStax driver in mapper code generator - does this need to be changed?

@hhughes
Copy link
Contributor

hhughes commented Sep 29, 2023

Python-script for filtering out common license change hunks from the diff: license_change_diff_filter.py

Copy link
Contributor

@hhughes hhughes left a comment

Choose a reason for hiding this comment

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

One remaining question about DataStax branding in SingleFileCodeGenerator.java, otherwise LGTM

@hhughes hhughes self-requested a review September 29, 2023 23:40
Copy link
Contributor

@hhughes hhughes left a comment

Choose a reason for hiding this comment

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

Re-approving this one

@michaelsembwever
Copy link
Member Author

All merged.

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.

2 participants