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

add more in createCollection #605

Merged
merged 5 commits into from
Nov 13, 2023
Merged

add more in createCollection #605

merged 5 commits into from
Nov 13, 2023

Conversation

johnsmartco
Copy link
Contributor

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@johnsmartco johnsmartco requested a review from a team as a code owner October 30, 2023 18:36
@johnsmartco johnsmartco self-assigned this Oct 30, 2023
@johnsmartco johnsmartco added this to the Ready for Review milestone Oct 30, 2023
vkarpov15
vkarpov15 reviewed Oct 31, 2023
View reviewed changes
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

A couple of comments for readability and consistency, but overall looks good

docs/jsonapi-spec.md Outdated Show resolved Hide resolved
docs/jsonapi-spec.md Outdated Show resolved Hide resolved
docs/jsonapi-spec.md Outdated Show resolved Hide resolved
docs/jsonapi-spec.md Outdated Show resolved Hide resolved
docs/jsonapi-spec.md Outdated Show resolved Hide resolved
@johnsmartco
Copy link
Contributor Author

@vkarpov15 , many thanks for the Spec feedback. I'll inc asap.

tatu-at-datastax
tatu-at-datastax approved these changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@tatu-at-datastax tatu-at-datastax left a comment

Choose a reason for hiding this comment

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

LGTM.
Cannot verify details of metric but I assume these come from other documentation; seem valid.

@tatu-at-datastax
Copy link
Contributor

@johnsmartco Needs force commit since CI won't run, will do.

@tatu-at-datastax tatu-at-datastax merged commit d63fb98 into main Nov 13, 2023
@tatu-at-datastax tatu-at-datastax deleted the johnsmartco-patch-1 branch November 13, 2023 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tatu-at-datastax tatu-at-datastax tatu-at-datastax approved these changes

@maheshrajamani maheshrajamani Awaiting requested review from maheshrajamani

@vkarpov15 vkarpov15 Awaiting requested review from vkarpov15

Assignees

@johnsmartco johnsmartco

Labels
None yet
Projects
None yet
Milestone
Ready for Review
Development

Successfully merging this pull request may close these issues.
4 participants
@johnsmartco @tatu-at-datastax @vkarpov15 @maheshrajamani