Skip to content

Conversation

@marandaneto
Copy link
Member

💡 Motivation and Context

#skip-changelog

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

@marandaneto marandaneto requested a review from a team as a code owner January 2, 2026 16:39
@marandaneto marandaneto requested a review from a team January 5, 2026 14:40
Copy link
Contributor

@dmarticus dmarticus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I trust your gut on this, so feel free to merge.

Fwiw, I ran this through claude code, and it gave some more feedback on improving the following areas, and it gave me the following feedback.

Suggestions for Improvement

  1. Add Common Pitfalls / Gotchas
    Claude benefits a lot from knowing what not to do:
## Common Pitfalls
- Don't use `@available` checks without also adding the platform to Package.swift's platforms array
- The SDK must remain thread-safe - all public APIs should be callable from any thread
- Avoid adding new dependencies - the SDK aims to stay lightweight
- Session recording has different availability per platform - check `#if os()` guards
- Feature flag evaluation must work offline - don't assume network availability
  1. Add Architecture Context
    Help Claude understand why things are structured the way they are:
## Architecture Notes
- The SDK uses a queue-based architecture for event batching
- Feature flags are cached locally and evaluated synchronously
- Session recording captures view hierarchy snapshots, not video
- The main `PostHog` class is a singleton accessed via `PostHog.shared`
  1. Add API Design Principles
    Since this is a public SDK:
## API Design Principles
- Public API changes require careful consideration for backwards compatibility
- Prefer optional parameters with sensible defaults over method overloads
- All public methods should have documentation comments
- Deprecated methods should use `@available(*, deprecated, message:)` with migration guidance
  1. Add Testing Patterns
    More specific guidance for the test framework you're using:
## Testing Patterns
- Use `describe`/`context`/`it` blocks (Quick syntax)
- Mock network with `stub(condition: isHost("us.i.posthog.com"))` pattern
- Use `waitUntil(timeout:)` for async assertions
- Test files should mirror source structure: `PostHog/Foo.swift``PostHogTests/FooTest.swift`
  1. Add Key Files Reference
    Help Claude find the important stuff quickly:
## Key Files
- `PostHog/PostHog.swift` - Main public interface, start here for API changes
- `PostHog/PostHogConfig.swift` - Configuration options
- `PostHog/PostHogFeatureFlags.swift` - Feature flag implementation
- `PostHog/PostHogQueue.swift` - Event batching and persistence
- `PostHog/PostHogSessionManager.swift` - Session handling 
  1. Add Platform-Specific Notes
## Platform Considerations
- **watchOS**: No session recording, limited lifecycle events
- **visionOS**: Relatively new, test thoroughly
- **macOS**: AppKit vs SwiftUI lifecycle differences
- **tvOS**: No touch-based gesture capture
  1. Add Example Patterns
    Show Claude how you want common tasks done:
## Code Examples

### Adding a new public method
```swift
/// Brief description of what this does.
/// - Parameter name: Description of parameter
/// - Returns: Description of return value
@objc public func methodName(param: Type) -> ReturnType {
    // Implementation
}

Adding platform-specific code

#if os(iOS) || os(tvOS)
    // iOS/tvOS specific implementation
#endif

8. Missing Practical Details
A few things that would help:
```markdown
## PR/Commit Guidelines
- Commits should be atomic and describe the "why"
- Breaking changes need BREAKING CHANGE in commit body
- Update CHANGELOG.md for user-facing changes

## Debugging Tips
- Enable verbose logging with `PostHogConfig().debug = true`
- Check `PostHog/Utils/Logging.swift` for log levels

Some of this is probably too general (e.g. I'd double-check the actual relevant files, etc), and you could take or leave it, but figured I'd share what I found.

@marandaneto
Copy link
Member Author

some of them do make sense @dmarticus thanks
i will let @ioannisj take a look since hes more active than me on this repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants