-
Notifications
You must be signed in to change notification settings - Fork 26
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
Release version 1.2.38 #2027
Release version 1.2.38 #2027
Conversation
WalkthroughThe pull request updates the project version from 1.2.37 to 1.2.38 across multiple files. In the changelog, a new entry for 1.2.38 is added with two notable features—NSCopying support for ART*ChannelOptions and a new API for Ably-authored SDKs. Alongside these additions, version references in documentation, scripts, source files, and tests have been updated, ensuring consistency throughout the project. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChannelOptions as ART*ChannelOptions
participant SDK as AblySDK
Client->>ChannelOptions: Create channel options
Note right of ChannelOptions: NSCopying support added
Client->>ChannelOptions: Request a copy of options
ChannelOptions-->>Client: Return copied options
Client->>SDK: Invoke new API with channel options copy
SDK-->>Client: Process and return response
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
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 (1)
CHANGELOG.md (1)
7-7
: Markdown Lint Suggestion (MD034 - No Bare URLs)
The change log section uses bare URLs in the descriptive bullet points and the “Full Changelog” link (e.g., on lines 7, 8, and 10). For improved readability and to adhere to markdown style guidelines, consider converting these bare URLs into reference-style links or inline links. For example, you might change:to something like:
- "… by @lawrence-forooghian in #2018"
This small change would enhance consistency and reduce markdownlint warnings.
Also applies to: 8-8, 10-10
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md
(1 hunks)README.md
(2 hunks)Scripts/jazzy.sh
(1 hunks)Source/ARTClientInformation.m
(1 hunks)Test/Tests/ARTDefaultTests.swift
(1 hunks)Test/Tests/ClientInformationTests.swift
(3 hunks)Test/Tests/RealtimeClientConnectionTests.swift
(1 hunks)Test/Tests/RestClientTests.swift
(1 hunks)Version.xcconfig
(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- Test/Tests/ARTDefaultTests.swift
- Version.xcconfig
- Scripts/jazzy.sh
- Source/ARTClientInformation.m
- README.md
- Test/Tests/RestClientTests.swift
- Test/Tests/RealtimeClientConnectionTests.swift
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
8-8: Bare URL used
null
(MD034, no-bare-urls)
10-10: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: check (macOS, test_macOS)
- GitHub Check: check (tvOS, test_tvOS17_2)
- GitHub Check: check (iOS, test_iOS17_2)
- GitHub Check: build
- GitHub Check: check
- GitHub Check: check
🔇 Additional comments (4)
Test/Tests/ClientInformationTests.swift (3)
12-12
: LGTM! Version string correctly updated.The version string has been properly updated to "1.2.38" in the test assertion.
30-30
: LGTM! Version string correctly updated.The version string has been properly updated to "1.2.38" in the expected agent identifier.
45-45
: LGTM! Version string correctly updated.The version string has been properly updated to "1.2.38" in the expected agent identifier with additional agents.
CHANGELOG.md (1)
3-10
: New Version Entry for 1.2.38 Added
The updated section for version 1.2.38 is clear and correctly formatted. It includes a proper header with a clickable link (e.g. “1.2.38”) and a concise bullet list describing the major changes:
• NSCopying support to ART*ChannelOptions
• Introduction of the new API for Ably-authored SDKsThis meets the PR objective for the new release and provides immediate context for users.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
8-8: Bare URL used
null(MD034, no-bare-urls)
10-10: Bare URL used
null(MD034, no-bare-urls)
0e3790d
to
2ccefe4
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
🔭 Outside diff range comments (1)
Scripts/set-version.sh (1)
15-20
: Critical Bug: Incorrect Conditional Check for NEW_VERSION VariableThe condition at line 17 uses a literal string ("NEW_VERSION") instead of referencing the variable. This always evaluates to false, so the script won’t properly validate that a version argument was provided. Please update the conditional to use "$NEW_VERSION" so that it correctly checks for an empty string.
Proposed fix:
-if [[ -z "NEW_VERSION" ]]; then +if [[ -z "$NEW_VERSION" ]]; then🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 17-17: Argument to -z is always false due to literal strings.
(SC2157)
🧹 Nitpick comments (2)
Scripts/set-version.sh (1)
42-44
: Nitpick: Sed Command QuotingThe sed command used for updating the version string is functional; however, the complex quoting can be error-prone if version strings ever contain regex-special characters. For improved clarity and robustness, consider simplifying or documenting this command further.
CHANGELOG.md (1)
10-10
: Full Changelog Link
The entry provides a link labeled "Full Changelog: 1.2.37...1.2.38" which is very useful for users wanting to review the complete set of changes between versions.
Note: Static analysis (markdownlint MD034) flags the use of bare URLs in this line. While it is common practice in changelogs to list URLs in plain form, you may consider wrapping the URL in angle brackets (e.g., https://...) if strict markdown linting is desired.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
10-10: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md
(1 hunks)README.md
(2 hunks)Scripts/jazzy.sh
(1 hunks)Scripts/set-version.sh
(1 hunks)Source/ARTClientInformation.m
(1 hunks)Test/Tests/ARTDefaultTests.swift
(1 hunks)Test/Tests/ClientInformationTests.swift
(3 hunks)Test/Tests/RealtimeClientConnectionTests.swift
(1 hunks)Test/Tests/RestClientTests.swift
(1 hunks)Test/Tests/WrapperSDKProxyTests.swift
(2 hunks)Version.xcconfig
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- Test/Tests/WrapperSDKProxyTests.swift
🚧 Files skipped from review as they are similar to previous changes (8)
- Test/Tests/ARTDefaultTests.swift
- Version.xcconfig
- Source/ARTClientInformation.m
- Scripts/jazzy.sh
- Test/Tests/ClientInformationTests.swift
- README.md
- Test/Tests/RestClientTests.swift
- Test/Tests/RealtimeClientConnectionTests.swift
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
7-7: Bare URL used
null
(MD034, no-bare-urls)
8-8: Bare URL used
null
(MD034, no-bare-urls)
10-10: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: check (macOS, test_macOS)
- GitHub Check: check (tvOS, test_tvOS17_2)
- GitHub Check: check (iOS, test_iOS17_2)
- GitHub Check: build
- GitHub Check: check
- GitHub Check: check
🔇 Additional comments (5)
Scripts/set-version.sh (2)
29-38
: Enhancement: New Test File Inclusion in version updateThe addition of "Test/Tests/WrapperSDKProxyTests.swift" to the other_files array is a positive change that helps maintain version consistency across all relevant files in the project. Ensure that this file exists and contains the expected version reference for the update process.
26-28
: Suggestion: Verify Git-Tracked File ConsistencyAt line 26 you update the version configuration file using "$VERSION_CONFIG_FILE" (presumably defined in version-constants.sh), but subsequently add "Version.xcconfig" explicitly (line 27). Make sure that "Version.xcconfig" is indeed the same file as referenced by $VERSION_CONFIG_FILE to avoid inconsistencies.
CHANGELOG.md (3)
3-4
: New Release Entry for Version 1.2.38
The new release entry is introduced correctly using a level‑2 heading, and the associated GitHub link (to the 1.2.38 tree) follows the established formatting in the changelog. This clearly marks the beginning of the latest release notes.
5-8
: "What's Changed" Section for 1.2.38
The "What's Changed" section lists the two key updates:
• AddingNSCopying
support toART*ChannelOptions
• Introducing a new API for Ably‑authored SDKs
Each bullet point correctly attributes the contributions (by @lawrence-forooghian) and references the relevant pull requests. This provides clear and traceable context for the changes.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Bare URL used
null(MD034, no-bare-urls)
8-8: Bare URL used
null(MD034, no-bare-urls)
11-11
: Spacing and Layout
An empty line following the Full Changelog link ensures good visual separation between the new release entry and the previous versions. The overall layout is consistent with earlier entries in the file.
Failures are not related to contents of this PR; merging. |
Summary by CodeRabbit