Remove fancy quotes #95
Conversation
Due to LLM limitations, LLM coding assistants frequently steamroller these and convert them into straight quotes. We have two choices: fight them at every step and keep reverting their changes, or just give up on fancy quotes. I think that realistically, we wouldn't do the first one, and would instead just keep introducing noise into diffs.
WalkthroughThis PR normalizes typographic apostrophes and quotation marks to ASCII equivalents across configuration files, source code comments, documentation, and test files. All changes are cosmetic textual updates with no functional modifications to logic, control flow, or public APIs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes This is a homogeneous, large-scale cosmetic change—the same type of apostrophe/quote normalization applied consistently across many files. While the file count is substantial, the repetitive nature and lack of functional changes reduce review complexity. Verification focuses on ensuring consistency of the transformation rather than evaluating distinct logic in each file. Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/BuildTool/BuildTool.swift (1)
167-171: Fix “Optional(…)” leaking into $GITHUB_OUTPUT.String(data:…, encoding: .utf8) returns an optional; the current interpolation yields “matrix=Optional("…")”. Use non‑optional decoding.
- // I'm assuming the JSONSerialization output has no newlines - let keyValue = try "matrix=\(String(data: JSONSerialization.data(withJSONObject: matrix), encoding: .utf8))" + // JSONSerialization produces UTF-8 data; decode without Optional + let jsonData = try JSONSerialization.data(withJSONObject: matrix) + let keyValue = "matrix=\(String(decoding: jsonData, as: UTF8.self))"
🧹 Nitpick comments (4)
Sources/BuildTool/DestinationFetcher.swift (1)
14-15: Optional: cap error payload size for readability.The failure message prints the entire devices dictionary; for large outputs this can flood logs. Consider logging only keys/counts.
- throw Error.simulatorLookupFailed(message: "Couldn't find a simulator with runtime \(runtimeIdentifier) and device type \(deviceTypeIdentifier); available devices are \(simctlAvailableDevicesOutput.devices)") + let availableRuntimes = Array(simctlAvailableDevicesOutput.devices.keys).sorted() + throw Error.simulatorLookupFailed( + message: "Couldn't find simulator with runtime \(runtimeIdentifier) and device type \(deviceTypeIdentifier); available runtimes: \(availableRuntimes.joined(separator: \", \")), devices count: \(simctlAvailableDevicesOutput.devices.values.flatMap { $0 }.count)" + )Sources/BuildTool/BuildTool.swift (2)
212-214: Doc comment has trailing “///”.Ends with “kept in sync.///”. Drop the extra slashes.
- /// Checks that the Swift version specified by the `Package.swift`'s `"swift-tools-version"` matches that in the `.swift-version` file (which is used to tell SwiftFormat the minimum version of Swift supported by our code). Per [SwiftFormat#1496](https://github.com/nicklockwood/SwiftFormat/issues/1496) it's currently our responsibility to make sure they're kept in sync./// + /// Checks that the Swift version specified by the `Package.swift`'s `"swift-tools-version"` matches that in the `.swift-version` file (which is used to tell SwiftFormat the minimum version of Swift supported by our code). Per [SwiftFormat#1496](https://github.com/nicklockwood/SwiftFormat/issues/1496) it's currently our responsibility to make sure they're kept in sync.
340-399: Regex only allows single‑digit m/n in @specOneOf.The pattern uses
(\d)?for index/total, so 10/12 won’t parse. Widen to(\d+)for robustness.- let conformanceTagSourceLineRegex = /^\s+\/\/ @spec(OneOf|Partial|Untested|NotApplicable)?(?:\((\d)?\/(\d)?\))? (.*?)(?: - (.*))?$/ + let conformanceTagSourceLineRegex = /^\s+\/\/ @spec(OneOf|Partial|Untested|NotApplicable)?(?:\((\d+)\/(\d+)\))? (.*?)(?: - (.*))?$/Sources/BuildTool/XcodeRunner.swift (1)
4-5: Add explicit ACL to satisfy explicit_acl guidelineDeclare the access level on the type.
Apply:
-enum XcodeRunner { +internal enum XcodeRunner {As per coding guidelines.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
.github/workflows/check.yaml(5 hunks).prettierignore(1 hunks).swiftlint.yml(4 hunks)CONTRIBUTING.md(2 hunks)Sources/AblyLiveObjects/.swiftformat(1 hunks)Sources/AblyLiveObjects/.swiftlint.yml(1 hunks)Sources/AblyLiveObjects/Utility/Logger.swift(1 hunks)Sources/BuildTool/BuildTool.swift(13 hunks)Sources/BuildTool/DestinationFetcher.swift(1 hunks)Sources/BuildTool/ProcessRunner.swift(1 hunks)Sources/BuildTool/XcodeRunner.swift(1 hunks)Tests/AblyLiveObjectsTests/Helpers/TestLogger.swift(1 hunks)Tests/AblyLiveObjectsTests/JSONValueTests.swift(1 hunks)Tests/AblyLiveObjectsTests/WireValueTests.swift(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)
**/*.swift: Specify an explicit access control level (SwiftLint explicit_acl) for all declarations in Swift code (tests are exempt)
When extending a type, put the access level on the extension declaration rather than on each member (tests are exempt)
Prefer implicit .init(...) when the type can be inferred in initializer expressions
Prefer enum case shorthand (.caseName) when the type can be inferred
For JSONValue or WireValue, prefer using literal syntax via ExpressibleBy*Literal where possible
Prefer Swift raw string literals for JSON strings instead of escaping double quotes
When an array literal begins with an initializer expression, place the initializer on the line after the opening bracket
Files:
Sources/BuildTool/XcodeRunner.swiftTests/AblyLiveObjectsTests/WireValueTests.swiftSources/BuildTool/DestinationFetcher.swiftSources/AblyLiveObjects/Utility/Logger.swiftSources/BuildTool/ProcessRunner.swiftTests/AblyLiveObjectsTests/Helpers/TestLogger.swiftTests/AblyLiveObjectsTests/JSONValueTests.swiftSources/BuildTool/BuildTool.swift
Sources/**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
For
testsOnly_property declarations, do not add generic explanatory comments (their meaning is understood)
Files:
Sources/BuildTool/XcodeRunner.swiftSources/BuildTool/DestinationFetcher.swiftSources/AblyLiveObjects/Utility/Logger.swiftSources/BuildTool/ProcessRunner.swiftSources/BuildTool/BuildTool.swift
Tests/**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)
Tests/**/*.swift: Use the Swift Testing framework (import Testing), not XCTest, in test files
Do not usefatalErrorfor expectation failures; prefer Swift Testing’s#require
Only add labels to test cases or suites when the label differs from the suite struct or test method name
Tag tests per CONTRIBUTING.md’s "Attributing tests to a spec point" with exact comment format; distinguish@specvs@specPartial; do not repeat@specfor the same spec point
Add comments in tests to clarify when certain test data is irrelevant to the scenario
In tests, import Ably usingimport Ably
In tests, import AblyLiveObjects using@testable import AblyLiveObjects
In tests, import_AblyPluginSupportPrivateusingimport _AblyPluginSupportPrivate(do not useinternal import)
When passing a logger to internal components in tests, useTestLogger()
When unwrapping optionals in tests, prefer#requireoverguard let
Files:
Tests/AblyLiveObjectsTests/WireValueTests.swiftTests/AblyLiveObjectsTests/Helpers/TestLogger.swiftTests/AblyLiveObjectsTests/JSONValueTests.swift
Sources/AblyLiveObjects/**/*.swift
📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)
In AblyLiveObjects library (non-test) code, import modules as: Ably with
import Ably, and _AblyPluginSupportPrivate withinternal import _AblyPluginSupportPrivate
Files:
Sources/AblyLiveObjects/Utility/Logger.swift
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check-documentation
- GitHub Check: generate-matrices
🔇 Additional comments (14)
Sources/AblyLiveObjects/.swiftformat (1)
1-1: Consistent with PR objective to remove fancy quotes.This cosmetic update normalizes the typographic apostrophe to ASCII equivalent, which directly addresses the stated PR goal of preventing LLM-induced diff noise.
Sources/BuildTool/ProcessRunner.swift (1)
7-7: Consistent with PR objective; comment formatting unchanged.The typographic apostrophes have been normalized to ASCII equivalents without affecting the comment's meaning or code functionality.
Tests/AblyLiveObjectsTests/JSONValueTests.swift (1)
104-104: Consistent with PR objective; test logic unchanged.The typographic apostrophe in the comment has been normalized to ASCII equivalent. No changes to test assertions or behavior.
.prettierignore (1)
1-1: Consistent with PR objective; configuration behavior unchanged.The typographic apostrophe in the comment has been normalized to ASCII. File functionality and Prettier exclusion rules remain unchanged.
Tests/AblyLiveObjectsTests/WireValueTests.swift (2)
223-223: Consistent with PR objective; test logic unchanged.The typographic apostrophe in the comment has been normalized to ASCII. Test assertions and behavior remain unchanged.
277-277: Consistent with PR objective; test logic unchanged.The typographic apostrophe in the comment has been normalized to ASCII. Test assertions and behavior remain unchanged.
Sources/AblyLiveObjects/Utility/Logger.swift (1)
5-5: Consistent with PR objective; documentation clarity preserved.The typographic apostrophe in the documentation comment has been normalized to ASCII. Type definition and all code implementations remain unchanged.
.github/workflows/check.yaml (1)
23-23: Consistent with PR objective; workflow behavior unchanged.Five instances of typographic apostrophes in workflow comments have been normalized to ASCII equivalents. CI/CD job definitions, step implementations, and workflow execution logic remain unchanged.
Also applies to: 52-52, 71-71, 152-152, 229-229
Tests/AblyLiveObjectsTests/Helpers/TestLogger.swift (1)
7-7: Consistent with PR objective; test logger functionality unchanged.The typographic apostrophe in the comment has been normalized to ASCII. TestLogger implementation and all logging behavior remain unchanged.
CONTRIBUTING.md (2)
39-41: LGTM: quote normalization only.ASCII apostrophes read well and avoid tooling churn. No semantic changes.
115-117: LGTM: consistent tag wording.Straight quotes in examples and CI note look good; matches the PR’s goal.
Also applies to: 119-121
.swiftlint.yml (1)
19-21: LGTM: comments normalized to ASCII apostrophes.Purely cosmetic; no rule or behavior changes.
Also applies to: 37-66, 67-78, 79-85
Sources/AblyLiveObjects/.swiftlint.yml (1)
2-3: LGTM: comment normalization.No config changes; safe.
Sources/BuildTool/BuildTool.swift (1)
130-148: LGTM: ASCII quote normalization across comments/docstrings.All these hunks are text‑only; no behavior change. Matches the repo’s decision to avoid curly quotes.
Also applies to: 236-241, 451-454, 628-633, 691-694, 772-789, 815-833
Due to LLM limitations, LLM coding assistants frequently steamroller these and convert them into straight quotes. We have two choices: fight them at every step and keep reverting their changes, or just give up on fancy quotes. I think that realistically, we wouldn't do the first one, and would instead just keep introducing noise into diffs.
Summary by CodeRabbit