[PM-26809] Improve Claude and add Review changes Skill #2039
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||||||||
| # Bitwarden iOS - Claude Code Configuration | ||||||||||||
|
|
||||||||||||
| ## Project Context Files | ||||||||||||
|
|
||||||||||||
| **Read these files before reviewing to ensure that you fully understand the project and contributing guidelines** | ||||||||||||
|
|
||||||||||||
| 1. @README.md | ||||||||||||
| 2. @CONTRIBUTING.md | ||||||||||||
| 3. @.github/PULL_REQUEST_TEMPLATE.md | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
|
||||||||||||
| ## Project Structure Quick Reference | ||||||||||||
|
|
||||||||||||
| - **BitwardenShared/**: Main codebase (Core + UI layers) | ||||||||||||
| - **Core/**: Data layer (Models, Services, Repositories, Stores) | ||||||||||||
| - **Sourcery/**: Sourcery related files, including auto generated mocks. | ||||||||||||
| - **UI/**: Presentation layer (Coordinators, Processors, Views, State) | ||||||||||||
| - **AuthenticatorShared/**: Same as `BitwardenShared` but for Authenticator app. | ||||||||||||
| - **Domains**: Auth | Autofill | Platform | Tools | Vault | ||||||||||||
| - **Apps**: The projects contain two main apps: "Password Manager" and "Authenticator". | ||||||||||||
| - ** | ||||||||||||
|
||||||||||||
| - **Main Password Manager targets**: Bitwarden, BitwardenActionExtension, BitwardenAutoFillExtension, BitwardenShareExtension, BitwardenWatchApp, BitwardenWatchWidgetExtension | ||||||||||||
| - **Main Authenticator target**: Authenticator | ||||||||||||
| - **Shared apps frameworks**: BitwardenKit, BitwardenKitMocks, BitwardenResources, AuthenticatorBridgeKit, AuthenticatorBridgeKitMocks, Networking, TestHelpers | ||||||||||||
|
|
||||||||||||
| ## Critical Rules | ||||||||||||
|
||||||||||||
|
|
||||||||||||
| - **NEVER** install third-party libaries unless explicitly told to. | ||||||||||||
fedemkr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||
| - **CRITICAL**: new encryption logic should not be added to this repo. | ||||||||||||
| - **NEVER** send unencrypted vault data to API services | ||||||||||||
| - **NEVER** commit secrets, credentials, or sensitive information. | ||||||||||||
| - **NEVER** log decrypted data, encryption keys, or PII | ||||||||||||
| - No vault data in error messages or console logs | ||||||||||||
| - **ALWAYS** Read "iOS Client Architecture" and "Code Style" in References before answer. | ||||||||||||
|
|
||||||||||||
| ## Common Patterns | ||||||||||||
|
|
||||||||||||
| - **UI changes**: Always follow Coordinator → Processor → State → View flow | ||||||||||||
| - **Data access**: UI layer mostly uses Repositories (never Stores directly and scarsely Services) | ||||||||||||
fedemkr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||
| - **State mutations**: Only in Processors | ||||||||||||
| - **Navigation**: Coordinators handle navigation via Routes/Events enums | ||||||||||||
| - **Testing**: Test file goes in same folder as implementation (e.g., `FooProcessor.swift` + `FooProcessorTests.swift`) | ||||||||||||
|
||||||||||||
| - **Testing**: Test file goes in same folder as implementation (e.g., `FooProcessor.swift` + `FooProcessorTests.swift`) | |
| - **Testing**: Test files go in the same folder as the implementation (e.g., `FooProcessor.swift` + `FooProcessorTests.swift`). Multiple test files can be used for a single class to improve readability. |
There was a problem hiding this comment.
I can add a direct link to the testing docs explanation on contributing docs as in there, it's explained how test files are organized. Also will make it plural.
Outdated
There was a problem hiding this comment.
🤔 Part of me wonders if this will be sufficient for BWA, or if we need to call BWA out separately, or if we should work at getting some of these files actually migrated to BitwardenKit
There was a problem hiding this comment.
I will rewrite this to take into account both apps. Even though we could move some things into BitwardenKit there are some objects/services that are particular of each app so we will still need the specific app ServiceContainers; so we should still be documenting them.
fedemkr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
fedemkr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Can we elaborate on this?
Suggested change
| - Test files named `<TypeToTest>Tests.swift` in same folder | |
| Test files must be named `<TypeToTest>Tests.swift` and be located in the same folder as the file being tested. |
There was a problem hiding this comment.
This is repeated from the Common patterns section. I'm wondering if there's actually value in it being in both places. 🤔
Outdated
There was a problem hiding this comment.
⛏️ Can we elaborate more here, too?
Suggested change
| - Test in light mode, dark mode, and large dynamic type | |
| - Tests must pass in light mode, dark mode, and large dynamic type. |
Outdated
There was a problem hiding this comment.
🤔 It may be helpful to provide instructions on how to run tests.
Suggested change
| ### How to Run Tests | |
| - After making changes, run the tests for the relevant scheme in Xcode to ensure your changes have not broken anything. | |
| - You can also run tests from the command line using `xcodebuild`. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
I think it would be helpful to add a clear purpose statement so Claude has immediate context and directive. Something like
The bottom "Files prefixed with
@...", part is to clarify file notation since it varies throughout the doc. Some places, like L7-L9, are prefixed@while others use hyperlinks pointing to the file.Uh oh!
There was an error while loading. Please reload this page.
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.
Agree, I'll add that. I think I can just switch to use
@in all places as it's the preferred way on Claude docs to avoid stating it here.