Reviewing Changes

Reviewing Changes

Instructions

Follow this process to review code changes for Bitwarden iOS:

Step 1: Understand Context

Start with high-level assessment of the change's purpose and approach. Read PR/commit descriptions and understand what problem is being solved.

Step 2: Verify Compliance

Systematically check each area against Bitwarden standards documented in CLAUDE.md:

1. Architecture: Follow patterns in Docs/Architecture.md

   • Unidirectional data flow Coordinators + Processors (using SwiftUI)
   • Dependency Injection using ServiceContainer
   • Repository pattern and proper data flow

2. Style: Adhere to Code Style

   • Naming conventions, code organization, formatting
   • Swift/SwiftUI guidelines

3. Compilation: Analyze for potential build issues

   • Import statements and dependencies
   • Type safety and null safety
   • API compatibility and deprecation warnings
   • Resource/SDK references and requirements

4. Testing: Verify appropriate test coverage

   • Unit tests for business logic and utility functions
   • Snapshot/View inspector tests for user-facing features when applicable
   • Test coverage for edge cases and error scenarios

5. Security: Given Bitwarden's security-focused nature

   • Proper handling of sensitive data
   • Secure storage practices (Keychain)
   • Authentication and authorization patterns
   • Data encryption and decryption flows
   • Zero-knowledge architecture preservation

Step 3: Document Findings

Identify specific violations with file:line_number references. Be precise about locations.

Step 4: Provide Recommendations

Give actionable recommendations for improvements. Explain why changes are needed and suggest specific solutions.

Step 5: Flag Critical Issues

Highlight issues that must be addressed before merge. Distinguish between blockers and suggestions.

Step 6: Acknowledge Quality

Note well-implemented patterns (briefly, without elaboration). Keep positive feedback concise.

Review Anti-Patterns (DO NOT)

• Be nitpicky about linter-catchable style issues
• Review without understanding context - ask for clarification first
• Focus only on new code - check surrounding context for issues
• Request changes outside the scope of this changeset

Examples

Good Review Format

## Summary
This PR adds biometric authentication to the login flow, implementing unidirectional data flow pattern with proper state management.

## Critical Issues
- `BitwardenShared/UI/Auth/Login/LoginView.swift:25` - No `scrollView` added, user can't scroll through the view.
- `BitwardenShared/Core/Auth/Services/AuthService.swift:120` - You must not use `try!`, change it to `try` in a `do...catch` block or throwing function.

## Suggested Improvements
- Consider extracting biometric prompt logic to separate struct
- Add missing tests for biometric failure scenarios
- `BitwardenShared/UI/Auth/Login/LoginView.swift:43` - Consider using existing `BitwardenTextField` component

## Good Practices
- Proper comments documentation
- Comprehensive unit test coverage
- Clear separation of concerns

## Action Items
1. Add scroll view in `LoginView`
2. Change `try!` to `try` in `AuthService`
3. Consider adding tests for error flows

What to Focus On

DO focus on:

• Architecture violations (incorrect patterns)
• Security issues (data handling, encryption)
• Missing tests for critical paths
• Compilation risks (type safety, null safety)

DON'T focus on:

• Minor formatting (handled by linters)
• Personal preferences without architectural basis
• Issues outside the changeset scope