-
Notifications
You must be signed in to change notification settings - Fork 1
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
docs: README trim and getting started guide #104
base: main
Are you sure you want to change the base?
Conversation
We now treat our website documentation as the source-of-truth for docs, excluding inline documentation and comments. This change trims the README to remove detailed implementation information.
WalkthroughThe changes modify the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
41-47
: Enhance API Key Instructions Formatting
The nested bullet points under the "An Ably API key" section are clear and helpful. For improved readability and consistency, consider confirming that the indentation across these bullets aligns with your markdown style guide.
105-110
: Streamlined "Getting Started" Section – Consider Kotlin Naming Conventions
The new "Getting Started" section is concise and effectively instructs users on initializing the Chat SDK. One minor suggestion: while the sample function is exported asSendChatMessage()
, following Kotlin’s idiomatic naming (e.g.,sendChatMessage()
) would enhance consistency.
166-172
: Specify Language for Fenced Code Block
A fenced code block displaying expected logcat output is missing an explicit language specifier. To adhere to markdown linting best practices (MD040), consider adding a language hint (e.g.,plaintext
):-``` +```plaintextThis helps tools correctly format and highlight the block.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
166-166: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
166-166: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: check
- GitHub Check: build
- GitHub Check: coverage
🔇 Additional comments (1)
README.md (1)
113-161
: Review Sample Code Implementation
The Kotlin code snippet clearly demonstrates initializing the Ably Realtime client, creating the Chat client, and managing room subscriptions with coroutines. The inline comments make the functionality easy to follow. For production-level guidance, you might add a note about disposing subscriptions or handling errors, but for a getting-started guide, the current implementation is acceptable.
Code Coverage
|
Apart from this, You might also like to update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
105-108
: Enhance the "Getting Started" Section
The new "Getting Started" section offers a concise overview for initializing the Chat client and sending a message, which aligns with the PR objective to trim redundant details. It would be helpful to include a brief note that the provided code snippet must be executed within a coroutine scope to function correctly.
113-161
: Review theSendChatMessage
Function Implementation
The sample implementation ofSendChatMessage
clearly demonstrates how to initialize the Ably Realtime client, subscribe to connection and room status changes, send a message, and perform cleanup. For best practices in Kotlin, consider renaming the function to follow the lowerCamelCase convention (i.e.sendChatMessage
). Additionally, you might want to mention that in a production environment, proper error handling and subscription cancellation would be necessary.- suspend fun SendChatMessage() { + suspend fun sendChatMessage() {
166-173
: Specify Fenced Code Block Language
The fenced code block displaying the sample log output does not currently include a language identifier. Adding one (for example, using ```text) will enhance readability and ensure compliance with markdown linting rules (MD040).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
166-166: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md
(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
166-166: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: check
- GitHub Check: coverage
- GitHub Check: build
🔇 Additional comments (2)
README.md (2)
41-47
: Clarify API Key Setup Instructions
The added bullet points under the API key prerequisite improve clarity by guiding users on where to sign up, how to access the dashboard, and which capabilities they need. Ensure that the capability list remains in sync with the latest documentation on authentication capabilities.
177-180
: Streamlined Documentation Section
The updated Documentation section now explicitly guides users to the complete documentation on the official Ably website, which aligns well with the PR’s goal to reduce duplication in the README. This is clear and concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since current kotlin web doc snippets are not in right format, I am reluctant to approve this PR for now.
I have created and self-assigned issue for this -> ably/docs#2440
Once related PR is merged, I will approve this PR right away.
a537285
to
715bef0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
README.md (2)
121-121
: Function Naming Convention
The sample function is namedGetStartedWithChat()
, which deviates from Kotlin’s idiomatic lowerCamelCase for function names. Consider renaming it togetStartedWithChat()
to adhere to best practices.Example diff:
- suspend fun GetStartedWithChat() { + suspend fun getStartedWithChat() {
167-174
: Specify Language for Fenced Code Block
The fenced code block presenting the expected log output does not have a language specifier, which may impact syntax highlighting or markdown rendering in some tools. Consider adding a language tag (e.g.,text
) to the opening fence.Example diff:
-``` +```text🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
167-167: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CONTRIBUTING.md
(2 hunks)README.md
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CONTRIBUTING.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
167-167: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: check
- GitHub Check: coverage
🔇 Additional comments (4)
README.md (4)
41-47
: Clarify Prerequisite Bullets
The additional instructions for signing up and configuring the API key (including required capabilities) enhance clarity for new users. Please verify that the indentation and formatting remain consistent with the rest of the document.
105-110
: New "Getting Started" Section Introduction
The introductory text for the Getting Started section is concise and clearly sets upfront expectations for the tutorial. This aligns well with the PR’s objective to streamline the README.
112-163
: Review of the Kotlin Code Sample
The provided Kotlin code sample effectively demonstrates the key steps: creating an Ably Realtime client, initializing the Chat client, subscribing to connection and message events, attaching a room, sending a message, and then cleaning up. The inline comments improve comprehension. Ensure that this sample is maintained as the API evolves.
178-181
: Enhanced Documentation Section
The new "Documentation" section clearly directs users to the complete official documentation, addressing previous feedback about making the link more prominent.
This change adds a getting started guide to the README to guide users towards sending their first message using Ably Chat. This will eventually be replicated to the website dashboard.
715bef0
to
308ba6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
README.md (1)
112-163
: Well-structured sample code for initializing and using Chat SDK.
The Kotlin code snippet in thegetStartedWithChat()
function is clear and demonstrates the essential steps:
- Initialization of the Ably Realtime client with proper options.
- Creation of the Chat client.
- Subscription to connection and room status changes.
- Sending a message and waiting using
delay
.A couple of minor suggestions:
- Consider adding a brief note or comment about unsubscribing from the listeners (i.e.
connectionSubscription
,roomSubscription
, andmessageSubscription
) when used in a production context to prevent potential resource leaks.- Remember that since the function is
suspend
, it needs to be called from within a coroutine scope.Overall, this snippet effectively meets the "getting started" objective.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CONTRIBUTING.md
(2 hunks)README.md
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CONTRIBUTING.md
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: coverage
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (4)
README.md (4)
41-47
: Clear and concise prerequisite bullet points.
The added bullet items for signing up, using an API key, and ensuring the proper capabilities are set are clear and actionable. The formatting and content align well with the objective of streamlining the README for new users.
105-110
: Effective "Getting Started" introduction section.
The new "Getting Started" title and the accompanying description clearly set the stage for a quick-start guide. This revised wording succinctly informs users of the expected outcome and guides them to follow the simple instructions.
165-174
: Clear demonstration of expected log output.
The added section showing the logcat output helps set proper expectations for new users by illustrating what to look for. This reinforces the instructional aspect of the getting started guide.
178-180
: Straightforward documentation call-to-action.
The updated "Documentation" section with a prominent link to the official Ably Chat SDK docs is concise and directs users to further details if needed. This change supports the goal to streamline content and avoid duplication with the website documentation.
7cb92dc
to
4984a07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have created PR to update web docs ably/docs#2448, once it's approved and merged, we can merge this 👍
This change includes:
CHA-842
CHA-818
Summary by CodeRabbit
suspend fun getStartedWithChat()
.