[ECO-5105] docs: updated README.md
#65
Conversation
WalkthroughThe changes in the pull request primarily involve extensive modifications to the Changes
Assessment against linked issues
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 (
|
ttypic
force-pushed
the
ECO-5105/update-readme
branch
from
a14cd01 to
571425d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
README.md (2)
591-597: Clarify channel usage for different featuresThe table shows that Messages, Presence, and Occupancy all use the same channel (
<roomId>::$chat::$chatMessages). Consider adding a note explaining why these features share the same channel to avoid confusion.Add a note below the table explaining the channel sharing design decision.
35-35: Fix grammatical and typographical issuesThere are several minor issues that should be addressed:
- Line 35: "i.e" is missing a period
- Line 168: "defaults options" should be "default options"
- Line 585: "side-effects" should be "side effects"
Apply these corrections to improve the document's professionalism.
Also applies to: 168-168, 585-585
🧰 Tools
🪛 LanguageTool
[uncategorized] ~35-~35: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...(I_E)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~32-~32: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~35-~35: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~45-~45: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~47-~47: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~89-~89: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~112-~112: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[uncategorized] ~116-~116: Possible missing comma found.
Context: ...en) in production environments. To use Chat you must also set a [clientId](https:...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~168-~168: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~170-~170: Consider a shorter alternative to avoid wordiness.
Context: ...in/java/com/ably/chat/RoomOptions.kt). In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~201-~201: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~201-~201: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~207-~207: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~207-~207: Possible missing comma found.
Context: ...finished with a room, you may release it which allows the underlying resources t...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~213-~213: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~252-~252: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~281-~281: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ener, call unsubscribe to remove that listeners subscription and prevent it from receiv...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~294-~294: Possible missing comma found.
Context: ... messages object also exposes the get method which can be used to request historical...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~367-~367: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[misspelling] ~585-~585: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
README.md
46-46: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
48-48: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
50-50: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
89-89: null
Bare URL used
(MD034, no-bare-urls)
607-607: null
Bare URL used
(MD034, no-bare-urls)
6-6: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
ttypic
force-pushed
the
ECO-5105/update-readme
branch
from
571425d to
23cb40f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
README.md (1)
34-34: Fix grammatical and formatting issuesSeveral minor grammatical and formatting improvements can be made:
- Add missing period in "i.e." abbreviation
- Fix plural "options" to singular "option" in defaults description
- Remove wordiness in phrases like "in order to" and "completely finished"
- Add missing commas in compound sentences
Example fixes:
-Chat room occupancy, i.e total number +Chat room occupancy, i.e., total number -The defaults options for each feature +The default option for each feature -In order to use the same room +To use the same room -When you are completely finished with a room +When you are finished with a roomAlso applies to: 167-167, 169-169, 200-200, 206-206
🧰 Tools
🪛 LanguageTool
[uncategorized] ~34-~34: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...(I_E)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~31-~31: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~34-~34: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~44-~44: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~46-~46: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~88-~88: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~111-~111: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[uncategorized] ~115-~115: Possible missing comma found.
Context: ...en) in production environments. To use Chat you must also set a [clientId](https:...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~167-~167: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~169-~169: Consider a shorter alternative to avoid wordiness.
Context: ...in/java/com/ably/chat/RoomOptions.kt). In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~200-~200: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~200-~200: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~206-~206: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~206-~206: Possible missing comma found.
Context: ...finished with a room, you may release it which allows the underlying resources t...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~251-~251: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~293-~293: Possible missing comma found.
Context: ... messages object also exposes the get method which can be used to request historical...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~366-~366: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[misspelling] ~584-~584: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
README.md
45-45: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
47-47: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
49-49: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
88-88: null
Bare URL used
(MD034, no-bare-urls)
606-606: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (2)
README.md (2)
1-615: Documentation structure and content approved
The documentation is well-structured, comprehensive, and provides clear examples for all major features. The identified issues are minor and don't affect the overall quality of the documentation.
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~34-~34: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~44-~44: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~46-~46: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~88-~88: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~111-~111: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[uncategorized] ~115-~115: Possible missing comma found.
Context: ...en) in production environments. To use Chat you must also set a [clientId](https:...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~167-~167: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~169-~169: Consider a shorter alternative to avoid wordiness.
Context: ...in/java/com/ably/chat/RoomOptions.kt). In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~200-~200: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~200-~200: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~206-~206: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~206-~206: Possible missing comma found.
Context: ...finished with a room, you may release it which allows the underlying resources t...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~251-~251: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~293-~293: Possible missing comma found.
Context: ... messages object also exposes the get method which can be used to request historical...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~366-~366: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[misspelling] ~584-~584: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
45-45: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
47-47: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
49-49: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
88-88: null
Bare URL used
(MD034, no-bare-urls)
606-606: null
Bare URL used
(MD034, no-bare-urls)
101-106: 🛠️ Refactor suggestion
Improve security in code examples
The example shows API key directly in the code. While this is just an example, it's better to demonstrate secure practices.
Update the example to use environment variables:
val realtimeClient = AblyRealtime(
ClientOptions().apply {
- key = "<api-key>"
- clientId = "<client-id>"
+ key = System.getenv("ABLY_API_KEY") // Load from environment variable
+ clientId = System.getenv("ABLY_CLIENT_ID") // Load from environment variable
},
)Likely invalid or redundant comment.
| the [community reported Github issues](https://github.com/ably/ably-chat-kotlin/issues) or raise one yourself. | ||
|
|
||
| To see what has changed in recent versions, see the [changelog](CHANGELOG.md). |
There was a problem hiding this comment.
💡 Codebase verification
Fix broken changelog link and improve documentation formatting
- Remove or update the broken CHANGELOG.md link as the file does not exist in the repository
- Format the support URL as a proper Markdown link:
[our support site](http://support.ably.com/)
🔗 Analysis chain
Fix broken changelog link and improve URL formatting
- The CHANGELOG.md file referenced in the README.md does not exist in the repository, making it a broken link
- The support URL should be formatted as a proper Markdown link for better maintainability
Update the support URL to use proper Markdown formatting:
-Please visit http://support.ably.com/ for access to our knowledge base
+Please visit [our support site](http://support.ably.com/) for access to our knowledge base🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if CHANGELOG.md exists
if [ -f CHANGELOG.md ]; then
echo "CHANGELOG.md exists"
else
echo "CHANGELOG.md is missing"
fi
Length of output: 82
🧰 Tools
🪛 Markdownlint (0.35.0)
606-606: null
Bare URL used
(MD034, no-bare-urls)
| To use Chat you must also set a [`clientId`](https://ably.com/docs/auth/identified-clients) so that clients are | ||
| identifiable. If you are prototyping, you can use `java.util.UUID` to generate an ID. |
There was a problem hiding this comment.
| To use Chat you must also set a [`clientId`](https://ably.com/docs/auth/identified-clients) so that clients are | |
| identifiable. If you are prototyping, you can use `java.util.UUID` to generate an ID. | |
| To use Chat you must also set a [`clientId`](https://ably.com/docs/auth/identified-clients) so that clients are | |
| identifiable. If you are prototyping, you can use `java.util.UUID` to generate an ID. | |
| P.S. In most cases, you can set `clientId` to `userId`, provided that `userId` is unique for your application. |
There was a problem hiding this comment.
Don't want to overcomplicate here, we never mentioned userId before, I think it can confuse the SDK users. And not sure we should mention something about userId being unique.
There was a problem hiding this comment.
Although, we never mention it before, that's the whole point of having clientId.
From user perspective, they shouldn't randomly set clientId as UUID, rather it should be mapped to corresponding user or use userIddirectly ( to avoid mapping)
There was a problem hiding this comment.
That's what we do in case of laravel as well getSignedToken. If its not mentioned here, then it should be a part of the official chat docs.
There was a problem hiding this comment.
You can ignore this thread for now, I will start a different thread on slack.
since this has overall chat docs usage implications across SDKs
Cc @AndyTWF
| The second argument to `rooms.get` is a `RoomOptions` argument, which tells the Chat SDK what features you would like your room to use and | ||
| how they should be configured. |
There was a problem hiding this comment.
| The second argument to `rooms.get` is a `RoomOptions` argument, which tells the Chat SDK what features you would like your room to use and | |
| how they should be configured. | |
| The second argument for `rooms.get` is a `RoomOptions` parameter, which specifies the features you want your room to utilize and their configuration. | |
| P.S. You can also use `RoomOptions.default` to enable all room features with the default configuration. |
|
|
||
| ## Supported Platforms | ||
|
|
||
| - Android 24 and above | ||
| This SDK works on Android 7.0+ (API level 24+) and Java 8+. |
There was a problem hiding this comment.
While we support Android 7.0+ (API level 24+) and Java 8+.
We don't have working CI environment ( to back our claim) for java 8 or tests running on emulator API level 24+
Maybe we should add support/ create separate ticket for the same.
There was a problem hiding this comment.
I think we can address this later
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
README.md (1)
28-34: Fix grammatical issues in features sectionThere are a few grammatical issues that need attention:
- Missing comma after "features"
- "i.e" should be "i.e." with periods
Apply these corrections:
-This project is under development so we will be incrementally adding new features +This project is under development, so we will be incrementally adding new features -Chat room occupancy, i.e total number +Chat room occupancy, i.e., total number🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: Possible missing comma found.
Context: ...ed chat features This project is under development so we will be incrementally adding new ...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~31-~31: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....(MANY_NN_U)
[uncategorized] ~34-~34: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...(I_E)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~28-~28: Possible missing comma found.
Context: ...ed chat features This project is under development so we will be incrementally adding new ...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~31-~31: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~34-~34: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~44-~44: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~46-~46: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~88-~88: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~111-~111: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[grammar] ~167-~167: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~169-~169: Consider a shorter alternative to avoid wordiness.
Context: ...in/java/com/ably/chat/RoomOptions.kt). In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~200-~200: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~200-~200: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~206-~206: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~212-~212: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~251-~251: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~255-~255: Possible missing comma found.
Context: ... provides an onDiscontinuity handler. Here you can register a listener that will b...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~280-~280: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ener, call unsubscribe to remove that listeners subscription and prevent it from receiv...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~293-~293: Possible missing comma found.
Context: ... messages object also exposes the get method which can be used to request historical...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~366-~366: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[uncategorized] ~440-~440: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...s to start will reset the timer, so the clients typing status will remain active. ```k...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[misspelling] ~584-~584: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
README.md
45-45: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
47-47: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
49-49: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
88-88: null
Bare URL used
(MD034, no-bare-urls)
606-606: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (4)
README.md (4)
101-106: Use environment variables in code examples
The example shows API key directly in the code. While this is just an example, it's better to demonstrate secure practices.
606-609: Fix broken links and improve URL formatting
- The CHANGELOG.md link is broken as the file doesn't exist
- The support URL should be formatted as a proper Markdown link
🧰 Tools
🪛 Markdownlint (0.35.0)
606-606: null
Bare URL used
(MD034, no-bare-urls)
420-420:
Add prerequisite for typing indicators
The typing indicators section should mention that users need to be attached to the room before using typing indicators.
Add this note before the typing indicators section:
+> Note: To use typing indicators, you must first be [attached](#attaching-to-a-room) to the room.
+
Typing events allow you to inform others that a client is typingLikely invalid or redundant comment.
4-6:
Fix incorrect badge references and add version badge
The badges need the following updates:
- The license badge incorrectly points to "3scale/saas-operator"
- Add a version badge to show the current version
Apply this diff:
<img src="https://img.shields.io/badge/development_status-Private_Beta-ab7df8" alt="Development status" />
- <img src="https://badgen.net/github/license/3scale/saas-operator" alt="License" />
+ <img src="https://img.shields.io/github/license/ably-labs/ably-chat-kotlin" alt="License" />
+ <img src="https://img.shields.io/badge/version-0.0.1--beta-blue" alt="Version" />Likely invalid or redundant comment.
README.md
Outdated
| [//]: # (## Installation) | ||
|
|
||
| [//]: # () | ||
| [//]: # (The Ably Chat SDK is available on the Maven Central Repository. To include the dependency in your project, add the following to your `build.gradle` file:) | ||
|
|
||
| [//]: # () | ||
| [//]: # (For Groovy:) | ||
|
|
||
| [//]: # () | ||
| [//]: # (```groovy) | ||
|
|
||
| [//]: # (implementation 'com.ably.chat:android') | ||
|
|
||
| [//]: # (```) | ||
|
|
||
| [//]: # () | ||
| [//]: # (For Kotlin Script (`build.gradle.kts`):) | ||
|
|
||
| [//]: # () | ||
| [//]: # (```kotlin) | ||
|
|
||
| [//]: # (implementation("com.ably.chat:android")) | ||
|
|
||
| [//]: # (```) | ||
|
|
||
| [//]: # () | ||
| [//]: # (### Dependency on ably-android) | ||
|
|
||
| [//]: # () | ||
| [//]: # (Certain functionalities are powered by the [ably-android](https://github.com/ably/ably-java) library. ) | ||
|
|
||
| [//]: # (The `ably-android` library is included as an api dependency within the Chat SDK, so there is no need to manually add it to your project.) |
There was a problem hiding this comment.
💡 Codebase verification
Maven coordinates in README are incorrect and package is not published
The Maven coordinates in the commented installation section are incorrect. Based on the build files:
- The package is not yet published to Maven Central
- The current version in development is "0.0.1"
- The namespace/group ID is "com.ably.chat"
- The module name suggests the artifact ID should be "chat-android" instead of "android"
Keep the section commented out until the package is published, but update the coordinates to:
implementation 'com.ably.chat:chat-android:0.0.1'🔗 Analysis chain
Verify Maven coordinates before uncommenting
The installation section is commented out pending Maven release. Ensure the Maven coordinates are correct before uncommenting:
- Group ID:
com.ably.chat - Artifact ID:
android
Let me verify if there are any build configuration files that might indicate the correct Maven coordinates.
Let me check the content of the build files to find the correct Maven coordinates.
Let me check the version catalog (libs.versions.toml) since the build files reference version and coordinates from there.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if the package exists on Maven Central
curl -s "https://search.maven.org/solrsearch/select?q=g:com.ably.chat+AND+a:android" | jq .
Length of output: 657
Script:
#!/bin/bash
# Search for build.gradle files
fd build.gradle
# Search for gradle.properties
fd gradle.properties
# Check if there's a Maven publish configuration
rg -A 5 "publishing" build.gradle || rg -A 5 "publishing" build.gradle.kts || true
Length of output: 262
Script:
#!/bin/bash
# Check main build.gradle.kts
cat build.gradle.kts
# Check chat-android/build.gradle.kts
cat chat-android/build.gradle.kts
# Check gradle.properties for any group/version info
cat gradle.properties
Length of output: 3936
Script:
#!/bin/bash
# Check for version catalog file
fd versions.toml
# If found in gradle directory, show its content
cat gradle/libs.versions.toml 2>/dev/null || true
Length of output: 3612
Co-authored-by: Andy Ford <[email protected]>
Co-authored-by: Andy Ford <[email protected]>
ttypic
force-pushed
the
ECO-5105/update-readme
branch
from
1b15546 to
5d6fc85
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
images/ably-chat-github-header.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~32-~32: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~35-~35: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~45-~45: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~47-~47: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~89-~89: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~112-~112: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[uncategorized] ~116-~116: Possible missing comma found.
Context: ...en) in production environments. To use Chat you must also set a [clientId](https:...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~173-~173: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~184-~184: Consider a shorter alternative to avoid wordiness.
Context: ...pingOptions(timeoutMs = 3_000)), ) ``` In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~215-~215: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~215-~215: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~221-~221: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~221-~221: Possible missing comma found.
Context: ...finished with a room, you may release it which allows the underlying resources t...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~227-~227: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~268-~268: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~272-~272: Possible missing comma found.
Context: ... provides an onDiscontinuity handler. Here you can register a listener that will b...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~297-~297: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ener, call unsubscribe to remove that listeners subscription and prevent it from receiv...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~310-~310: Possible missing comma found.
Context: ... messages object also exposes the get method which can be used to request historical...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~383-~383: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[uncategorized] ~460-~460: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...s to start will reset the timer, so the clients typing status will remain active. ```k...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[misspelling] ~604-~604: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
README.md
46-46: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
48-48: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
50-50: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
89-89: null
Bare URL used
(MD034, no-bare-urls)
626-626: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (4)
README.md (4)
626-629: Fix broken changelog link and improve documentation formatting
- Remove or update the broken CHANGELOG.md link as the file does not exist in the repository
- Format the support URL as a proper Markdown link
-Please visit http://support.ably.com/ for access to our knowledge base
+Please visit [our support site](http://support.ably.com/) for access to our knowledge base
-To see what has changed in recent versions, see the [changelog](CHANGELOG.md).🧰 Tools
🪛 Markdownlint (0.35.0)
626-626: null
Bare URL used
(MD034, no-bare-urls)
102-107: 🛠️ Refactor suggestion
Improve security in code examples
The example shows API key directly in the code. While this is just an example, it's better to demonstrate secure practices.
val realtimeClient = AblyRealtime(
ClientOptions().apply {
- key = "<api-key>"
- clientId = "<client-id>"
+ key = System.getenv("ABLY_API_KEY") // Load from environment variable
+ clientId = System.getenv("ABLY_CLIENT_ID") // Load from environment variable
},
)Likely invalid or redundant comment.
4-6:
Fix incorrect badge references
The badges have the following issues:
- The license badge incorrectly points to "3scale/saas-operator" repository
- Version badge shows a SNAPSHOT version which might need updating before release
Update the license badge to point to the correct repository:
- <img src="https://badgen.net/github/license/3scale/saas-operator" alt="License" />
+ <img src="https://img.shields.io/github/license/ably/ably-chat-kotlin" alt="License" />Likely invalid or redundant comment.
54-85: 🛠️ Refactor suggestion
Update Maven coordinates before uncommenting
The installation section is commented out pending Maven release. Update the coordinates to match the project configuration:
-implementation 'com.ably.chat:android'
+implementation 'com.ably.chat:chat-android:0.0.1'Likely invalid or redundant comment.
|
|
||
| ## Supported Platforms | ||
|
|
||
| - Android 24 and above | ||
| This SDK works on Android 7.0+ (API level 24+) and Java 8+. |
There was a problem hiding this comment.
💡 Codebase verification
Update README to match actual platform requirements
The build configuration shows:
- minSdk is set to 24 (Android 7.0+) which matches the README
- Java compatibility varies between modules:
- Some use Java 17
- Some use Java 8
- CI workflows only test with Java 17
The README should be updated to accurately reflect that Java 17 is required for development, while the runtime compatibility starts from Java 8.
Points needing attention:
- Update README.md line 25 to clarify Java requirements: "This SDK works on Android 7.0+ (API level 24+) and requires Java 17 for development, with runtime compatibility from Java 8+"
🔗 Analysis chain
Verify platform support claims
The README states support for "Android 7.0+ (API level 24+) and Java 8+", but we need to ensure our CI environment validates these claims.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check build configuration for Android and Java version requirements
fd "build.gradle" -x cat {} | grep -E "minSdk|sourceCompatibility|targetCompatibility"
# Check if CI workflow tests against minimum supported versions
fd "workflow" .github/workflows -x cat {} | grep -E "java-version|api-level"
Length of output: 421
Script:
#!/bin/bash
# Check CI workflow files for Android emulator setup and test matrix
fd "yml|yaml" .github/workflows -x cat {}
Length of output: 1316
Code Coverage
|
ttypic
force-pushed
the
ECO-5105/update-readme
branch
2 times, most recently
from
d285d91 to
3b36d9c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
README.md (1)
33-36: Fix grammatical issues in feature listThere are a few grammatical issues in the feature list:
- Line 33: Consider rephrasing "participation" to improve clarity
- Line 36: The abbreviation "i.e" should be "i.e."
-Chat rooms for 1:1, 1:many, many:1 and many:many participation. -Chat room occupancy, i.e total number of connections and presence members. +Chat rooms supporting 1:1, 1:many, many:1 and many:many interactions. +Chat room occupancy, i.e., total number of connections and presence members.🧰 Tools
🪛 LanguageTool
[grammar] ~33-~33: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....(MANY_NN_U)
[uncategorized] ~36-~36: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...(I_E)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
images/ably-chat-github-header.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~33-~33: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~36-~36: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~46-~46: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~48-~48: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~90-~90: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~113-~113: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[grammar] ~174-~174: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~185-~185: Consider a shorter alternative to avoid wordiness.
Context: ...pingOptions(timeoutMs = 3_000)), ) ``` In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~216-~216: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~216-~216: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~222-~222: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~228-~228: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~269-~269: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[style] ~384-~384: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[misspelling] ~605-~605: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
README.md
47-47: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
49-49: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
51-51: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
90-90: null
Bare URL used
(MD034, no-bare-urls)
627-627: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (4)
README.md (4)
66-66:
Update Maven coordinates
The Maven coordinates in the commented installation section need to be updated with the correct artifact ID.
-implementation 'com.ably.chat:chat-android'
+implementation 'com.ably.chat:chat-android:0.1.0-SNAPSHOT'Likely invalid or redundant comment.
5-5:
Fix incorrect license badge URL
The license badge currently points to "3scale/saas-operator" repository instead of this repository.
- <img src="https://badgen.net/github/license/3scale/saas-operator" alt="License" />
+ <img src="https://img.shields.io/github/license/ably/ably-chat-kotlin" alt="License" />Likely invalid or redundant comment.
103-108: 🛠️ Refactor suggestion
Improve security in code examples
The example shows API key directly in the code. While this is just an example, it's better to demonstrate secure practices.
val realtimeClient = AblyRealtime(
ClientOptions().apply {
- key = "<api-key>"
- clientId = "<client-id>"
+ key = System.getenv("ABLY_API_KEY") // Load from environment variable
+ clientId = System.getenv("ABLY_CLIENT_ID") // Load from environment variable
},
)Likely invalid or redundant comment.
627-630:
Fix broken changelog link and improve documentation formatting
- Remove or update the broken CHANGELOG.md link as the file does not exist in the repository
- Format the support URL as a proper Markdown link
-Please visit http://support.ably.com/ for access to our knowledge base
-To see what has changed in recent versions, see the [changelog](CHANGELOG.md).
+Please visit [our support site](https://support.ably.com/) for access to our knowledge base
+To see what has changed in recent versions, see the [releases page](https://github.com/ably/ably-chat-kotlin/releases).Likely invalid or redundant comment.
🧰 Tools
🪛 Markdownlint (0.35.0)
627-627: null
Bare URL used
(MD034, no-bare-urls)
|
|
||
| Get started using the [📚 documentation](https://ably.com/docs/products/chat). | ||
|
|
||
|  |
There was a problem hiding this comment.
Add missing header image
The header image reference is broken. Please add the image file to the repository at /images/ably-chat-github-header.png.
Would you like me to help create a GitHub issue to track adding the missing header image?
There was a problem hiding this comment.
Lgtm
PS. You can go through remaining comments, and update README accordingly
Particularly RoomOptions comment
ttypic
force-pushed
the
ECO-5105/update-readme
branch
from
3b36d9c to
e4f269f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
README.md (1)
36-36: Fix grammatical issues in feature listMinor grammatical corrections needed:
- Add a period after "i.e" in "i.e total number"
-Chat room occupancy, i.e total number of connections and presence members. +Chat room occupancy, i.e., total number of connections and presence members.🧰 Tools
🪛 LanguageTool
[uncategorized] ~36-~36: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...(I_E)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
images/ably-chat-github-header.pngis excluded by!**/*.png
📒 Files selected for processing (1)
README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~33-~33: Possible agreement error. The noun participation seems to be uncountable; consider using: “much participation”, “a good deal of participation”.
Context: ... rooms for 1:1, 1:many, many:1 and many:many participation. - Sending and receiving chat messages....
(MANY_NN_U)
[uncategorized] ~36-~36: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...at participants. - Chat room occupancy, i.e total number of connections and presenc...
(I_E)
[grammar] ~46-~46: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘account’) instead of another adjective.
Context: ...need the following prerequisites: - An Ably account - You can [sign up](https://ably.co...
(A_RB_NN)
[grammar] ~48-~48: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘API’) instead of another adjective.
Context: ...signup) to the generous free tier. - An Ably API key - Use the default or create a n...
(A_RB_NN)
[grammar] ~90-~90: You used an adverb (‘Ably’) instead of an adjective, or a noun (‘client’) instead of another adjective.
Context: ...t to your project.) ## Versioning The Ably client library follows [Semantic Versioning](h...
(A_RB_NN)
[typographical] ~113-~113: The word “however” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...he API Key directly for testing purposes, however it is strongly recommended that you use...
(HOWEVER_SENTENCE)
[uncategorized] ~117-~117: Possible missing comma found.
Context: ...en) in production environments. To use Chat you must also set a [clientId](https:...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~174-~174: You should probably use “option”.
Context: ...ubscription to, presence. The defaults options for each feature may be viewed [here](h...
(AGREEMENT_SENT_START)
[style] ~185-~185: Consider a shorter alternative to avoid wordiness.
Context: ...pingOptions(timeoutMs = 3_000)), ) ``` In order to use the same room but with different op...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~216-~216: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... any event listeners you have registered and they will begin to receive events again...
(COMMA_COMPOUND_SENTENCE)
[style] ~216-~216: ‘in the event that’ might be wordy. Consider a shorter alternative.
Context: ...they will begin to receive events again in the event that the room is re-attached. ### Releasing...
(EN_WORDINESS_PREMIUM_IN_THE_EVENT_THAT)
[style] ~222-~222: ‘completely finished’ might be wordy. Consider a shorter alternative.
Context: ...running 1:1 support chat). When you are completely finished with a room, you may release it which...
(EN_WORDINESS_PREMIUM_COMPLETELY_FINISHED)
[uncategorized] ~222-~222: Possible missing comma found.
Context: ...finished with a room, you may release it which allows the underlying resources t...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~228-~228: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...is called, the room will become unusable and you will need to get a new instance usi...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~269-~269: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nection, continuity cannot be guaranteed and you'll need to take steps to recover me...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~298-~298: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ener, call unsubscribe to remove that listeners subscription and prevent it from receiv...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~311-~311: Possible missing comma found.
Context: ... messages object also exposes the get method which can be used to request historical...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~384-~384: Consider shortening or rephrasing this to strengthen your wording.
Context: ...the presence data Updates allow you to make changes to the custom data associated with a prese...
(MAKE_CHANGES)
[misspelling] ~605-~605: Did you mean “side effects” (=adverse effect, unintended consequence)? Open compounds are not hyphenated.
Context: ...directly. Doing so may cause unintended side-effects in the Chat SDK. ### Channels Used Fo...
(AFFECT_EFFECT)
🪛 Markdownlint (0.35.0)
README.md
47-47: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
49-49: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
51-51: Expected: 2; Actual: 4
Unordered list indentation
(MD007, ul-indent)
90-90: null
Bare URL used
(MD034, no-bare-urls)
627-627: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (6)
README.md (6)
627-630: Fix broken changelog link and improve documentation formatting
- The CHANGELOG.md file referenced does not exist in the repository
- Format the support URL as a proper Markdown link
-Please visit http://support.ably.com/ for access to our knowledge base
+Please visit [our support site](http://support.ably.com/) for access to our knowledge base
-To see what has changed in recent versions, see the [changelog](CHANGELOG.md).🧰 Tools
🪛 Markdownlint (0.35.0)
627-627: null
Bare URL used
(MD034, no-bare-urls)
22-22:
Add missing header image
The header image reference is broken. The image file needs to be added to the repository at the specified path.
103-108:
Improve security in code examples
The example shows API key directly in the code. While this is just an example, it's better to demonstrate secure practices.
val realtimeClient = AblyRealtime(
ClientOptions().apply {
- key = "<api-key>"
- clientId = "<client-id>"
+ key = System.getenv("ABLY_API_KEY") // Load from environment variable
+ clientId = System.getenv("ABLY_CLIENT_ID") // Load from environment variable
},
)Likely invalid or redundant comment.
613-615: Fix channel mapping in documentation
The channels table shows the same channel (<roomId>::$chat::$chatMessages) for Messages, Presence, and Occupancy. Please verify if this is correct or if these features should use different channels.
✅ Verification successful
Fix incorrect channel mappings in documentation
Based on the code analysis:
- Messages use channel:
<roomId>::$chat::$chatMessages - Typing indicators use channel:
<roomId>::$chat::$typingIndicators - Reactions use channel:
<roomId>::$chat::$reactions - Presence uses the same channel as Messages (verified in Presence.kt:
override val channel: Channel = room.messages.channel) - Occupancy uses the same channel as Messages (verified in Occupancy.kt)
The documentation is correct in showing the same channel (<roomId>::$chat::$chatMessages) for Messages, Presence, and Occupancy since they all operate on the same underlying channel as evidenced in the source code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for channel definitions in the codebase
rg -A 2 "companion object.*Channel.*=" || true
Length of output: 47
Script:
#!/bin/bash
# Search for channel-related patterns in the codebase
rg -i "channel.*=.*chat.*messages" || true
rg -i "presence.*channel" || true
rg -i "occupancy.*channel" || true
Length of output: 770
Script:
#!/bin/bash
# Look for any documentation files that might contain channel information
fd -e md -e txt | xargs rg -i "channel|presence|occupancy" || true
Length of output: 3974
Script:
#!/bin/bash
# Search for any constants or configurations related to channels
rg -i "const.*channel" || true
rg -i "val.*channel" || true
Length of output: 10181
66-66:
Update Maven coordinates
The Maven coordinates in the commented installation section need to be updated with the correct artifact ID.
-implementation 'com.ably.chat:chat-android'
+implementation 'com.ably.chat:chat-android:0.1.0-SNAPSHOT'Likely invalid or redundant comment.
5-5:
Fix incorrect license badge reference
The license badge currently points to an unrelated repository (3scale/saas-operator).
- <img src="https://badgen.net/github/license/3scale/saas-operator" alt="License" />
+ <img src="https://img.shields.io/github/license/ably/ably-chat-kotlin" alt="License" />Likely invalid or redundant comment.
Updated readme for chat sdk
Summary by CodeRabbit
README.mdfor improved content organization and formatting.