[PM-28512] Update iOS Getting Started guide to remove explicit versions #711
Conversation
…e of Xcode and iOS Simulator versions to reference the repo files instead.
|
Claude finished @fedemkr's task —— View job Review completePR Title & Description: Well-written with clear objective and Jira reference. Summary: This PR successfully achieves its goal of removing hardcoded version references and linking to repository files for maintainability. The documentation improvements are solid, though there are minor issues to address. FindingsFinding 1: Minor grammatical issues in the Xcode version mismatch warning introduction (docs/getting-started/mobile/ios/index.md:64-65) Finding 2: Example Xcode version "26.0.1" in the warning output is unrealistic and potentially confusing (docs/getting-started/mobile/ios/index.md:69) Positive observationsThe approach of linking to repository files ( |
Deploying contributing-docs with
|
| Latest commit: |
e820d51
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cf6452c1.contributing-docs.pages.dev |
| Branch Preview URL: | https://pm-28512-update-ios-requirem.contributing-docs.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ``` | ||
|
|
||
| Also, if the Xcode version installed mismatches the expected one you will receive a warning when | ||
| the script finishes like this one which can help troubleshooting. |
There was a problem hiding this comment.
| the script finishes like this one which can help troubleshooting. | |
| Also, if the Xcode version installed mismatches the expected one, you will receive a warning when | |
| the script finishes like this one, which can help troubleshooting: |
Changes:
- Added comma after "expected one" (compound sentence structure)
- Added comma before "which" (non-restrictive clause)
- Added colon after "troubleshooting" (introduces the example that follows)
There was a problem hiding this comment.
Claude is half-correct here; there should be more commas, though I would actually break it up into two sentences:
Also, if the installed Xcode version does not match the expected version, you will receive a warning, which can help with troubleshooting. That warning looks like this:
I also changed "mismatch" to "do not match" because it sounds slightly better to my ear, but I'm not wedded to that.
However, "the script finishes like this one" is a pretty not-great dangling modifier IMO; Claude is incorrect, and there should be another comma between "finishes" and "like".
|
|
||
| ``` | ||
| 🟡 Xcode version mismatch! | ||
| Required version: 26.0.1 |
There was a problem hiding this comment.
💭 Sample version value concern: The example shows "Required version: 26.0.1" which appears unrealistic (Xcode 26 doesn't exist yet - current is 16.x).
Consider using a version number that matches reality to avoid confusion:
🟡 Xcode version mismatch!
Required version: 16.0.1
Current version: 16.4
This helps developers understand what actual version numbers look like and reduces potential confusion.
|
Great job! No new security vulnerabilities introduced in this pull request |
| ``` | ||
|
|
||
| Also, if the Xcode version installed mismatches the expected one you will receive a warning when | ||
| the script finishes like this one which can help troubleshooting. |
There was a problem hiding this comment.
Claude is half-correct here; there should be more commas, though I would actually break it up into two sentences:
Also, if the installed Xcode version does not match the expected version, you will receive a warning, which can help with troubleshooting. That warning looks like this:
I also changed "mismatch" to "do not match" because it sounds slightly better to my ear, but I'm not wedded to that.
However, "the script finishes like this one" is a pretty not-great dangling modifier IMO; Claude is incorrect, and there should be another comma between "finishes" and "like".
| 2. An iPhone 16 Pro simulator (iOS 18.1) set up | ||
| 1. [Xcode](https://developer.apple.com/xcode/) (using | ||
| [this](https://github.com/bitwarden/ios/blob/main/.xcode-version) version) | ||
| 2. iOS & watchOS simulator runtimes installed |
There was a problem hiding this comment.
🤔 I generally think of ampersands as being too casual for usage in sentences like this. I'd prefer this (and the one on the line before) to be explicit "and"s, but that's something of a personal style thing.
There was a problem hiding this comment.
@KatherineInCode Thanks for raising this, added the changes in #631
🎟️ Tracking
PM-28512
📔 Objective
Update iOS Getting Started guide to remove explicit reference of Xcode and iOS Simulator versions to reference the repo files instead which have the corresponding versions always up to date.
Also added a clarification for Xcode versions mismatch from the
bootstrap.shwhich can be useful for troubleshooting.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes