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.

Sign up for GitHub

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

Jump to bottom

Improve type mapping documentation for Cassandra connector #13833

Merged
merged 1 commit into from
Sep 5, 2022
Merged

Improve type mapping documentation for Cassandra connector #13833

merged 1 commit into from
Sep 5, 2022

Conversation

tlblessing
Copy link
Member

Description

Improve type mapping documentation for Cassandra connector.

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement to Cassandra connector documentation

How would you describe this change to a non-technical end user or system administrator?

Added data type mapping; reformatted table; added boilerplate headings and verbiage for consistency.

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@cla-bot cla-bot bot added the cla-signed label Aug 24, 2022
jhlodin
jhlodin reviewed Aug 24, 2022
View reviewed changes
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
@jhlodin
Copy link
Contributor

jhlodin commented Aug 24, 2022

Please squash commits

@tlblessing tlblessing marked this pull request as ready for review August 24, 2022 22:01
@tlblessing tlblessing requested a review from chen-ni August 25, 2022 20:21
@tlblessing tlblessing marked this pull request as draft August 30, 2022 18:47
@tlblessing tlblessing force-pushed the tb/cassandra-type-mapping branch from 95ca4d5 to f7f815c Compare August 31, 2022 14:37
@tlblessing
Copy link
Member Author

Please squash commits

done

@tlblessing tlblessing marked this pull request as ready for review August 31, 2022 14:40
@tlblessing tlblessing requested a review from jhlodin August 31, 2022 14:40
@tlblessing tlblessing force-pushed the tb/cassandra-type-mapping branch from f7f815c to 8214e35 Compare August 31, 2022 14:57
@tlblessing
Copy link
Member Author

@ebyhr I have incorporated @jhlodin's feedback but need your expertise on which duplicate mappings on the read side should be removed.

jhlodin
jhlodin approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@jhlodin jhlodin left a comment

Choose a reason for hiding this comment

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

Style looks good to me, just need @ebyhr or another SME to provide guidance on the table contents.

@jhlodin jhlodin requested a review from hashhar August 31, 2022 21:10
ebyhr
ebyhr reviewed Sep 1, 2022
View reviewed changes
Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Could you avoid putting one-to-many entries when you're not sure about the mapping (just putting one entry is enough)? Otherwise, we have to leave many "remove" comments and it delays a review process.

docs/src/main/sphinx/connector/cassandra.rst Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/connector/cassandra.rst Outdated Show resolved Hide resolved
@tlblessing tlblessing force-pushed the tb/cassandra-type-mapping branch from 8214e35 to c8e7ce3 Compare September 2, 2022 20:09
@tlblessing tlblessing requested a review from ebyhr September 2, 2022 20:10
@ebyhr ebyhr force-pushed the tb/cassandra-type-mapping branch from c8e7ce3 to be83b8e Compare September 5, 2022 00:37
@ebyhr ebyhr merged commit 692a5c3 into trinodb:master Sep 5, 2022
@ebyhr
Copy link
Member

ebyhr commented Sep 5, 2022

Merged, thanks!

@github-actions github-actions bot added this to the 395 milestone Sep 5, 2022
@colebow colebow mentioned this pull request Sep 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhlodin jhlodin jhlodin approved these changes

@hashhar hashhar Awaiting requested review from hashhar

@ebyhr ebyhr Awaiting requested review from ebyhr

Assignees
No one assigned
Labels
cla-signed docs
Milestone
395
Development

Successfully merging this pull request may close these issues.
3 participants
@tlblessing @jhlodin @ebyhr