Repository navigation
[PM-44082] docs: add brew bundle step to iOS getting started page. - #877
Conversation
Deploying contributing-docs with
|
| Latest commit: |
ef8f330
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://653fd219.contributing-docs.pages.dev |
| Branch Preview URL: | https://pm-44082-brew-bundle-step.contributing-docs.pages.dev |
7811b16 to
2fa13f6
Compare
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR updates Code Review DetailsNo findings met the confidence threshold for posting. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
KatherineInCode
left a comment
KatherineInCode
left a comment
There was a problem hiding this comment.
Nothing wrong per se, but just a few questions on things we could potentially streamline and improve further.
|
|
||
| ```sh | ||
| $ brew install mint | ||
| $ cd ios |
There was a problem hiding this comment.
🤔 is the cd ios necessary, since we also say "from the repository root"? It doesn't hurt, I suppose
|
|
||
| 2. Install [Mint](https://github.com/yonaskolb/mint): | ||
| 2. Install [Homebrew](https://brew.sh/) dependencies (including | ||
| [Mint](https://github.com/yonaskolb/mint)) from the repository root: |
There was a problem hiding this comment.
❓ I feel like calling out Mint is an artifact from when we only brew installed Mint. Maybe we should leave the mention off here, and migrate things later in the file to not call out Mint specifically?
There was a problem hiding this comment.
I like this suggestion. If we take the Mint callout out of step 2, does it still make sense to have the "alternative" Mint install instructions on line 38?
There was a problem hiding this comment.
I'd say it makes sense to remove it there, too. It's an artifact from when mint was the only thing we'd install with homebrew.
| ``` | ||
| $ rbenv install 3.4.4 | ||
| ``` | ||
| If you see an error that a Ruby version is not installed, or that you should run |
There was a problem hiding this comment.
🤔 While we use fastlane on CI, I don't know that we need to include it anymore for local discussion, unless there's something I'm missing? @vvolkgang you may want to weigh in.
There was a problem hiding this comment.
Yep, it should only needed if you need to work on fastlane stuff, @tomhartnett-bw let me know if that's not the case.
There was a problem hiding this comment.
Are we saying we should take out this troubleshooting comment about the Ruby version on lines 107–108, or clarify that step 4, beginning on line 80, applies to ci/automation and is not needed for local development?
There was a problem hiding this comment.
I think we should keep it in, but clarify that it's only needed if you're going to be doing local development of CI/automation flows.
Thanks for this feedback. I have opened a follow-up PR the address the feedback: #878 |
5 participants
🎟️ Tracking
PM-44082
📔 Objective
Update instructions on iOS Getting Started page to run
brew bundle.📸 Screenshots