chore(feature flags): code review follow-ups (documentation and bug fixes)#56
Merged
chore(feature flags): code review follow-ups (documentation and bug fixes)#56
Conversation
- Add Clone derive to LocalEvaluator for flexibility - Handle poisoned mutex gracefully in regex cache (use lock().ok()?) - Add date parsing edge case tests (empty strings, malformed dates, large values) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Documents FlagValue, InconclusiveMatchError, FeatureFlag, FeatureFlagFilters, FeatureFlagCondition, Property, MultivariateFilter, and MultivariateVariant. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Shows three patterns for handling errors: 1. Full match on error types 2. unwrap_or for simple defaults 3. Propagating errors with ? Co-Authored-By: Claude Opus 4.5 <[email protected]>
Document LocalEvaluationResponse, Cohort, FlagCache, LocalEvaluationConfig, FlagPoller, AsyncFlagPoller, and LocalEvaluator with rustdoc comments. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Document all Error variants and add module-level documentation to ClientOptions with usage examples. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Document the flag evaluation detail types with field-level documentation explaining each property's purpose. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Document the response enum variants and the hash_key and get_matching_variant functions used for flag bucketing. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replace "under development" notice with a proper introduction listing the SDK's capabilities. Co-Authored-By: Claude Opus 4.5 <[email protected]>
dmarticus
changed the title
Address Phil's code review feedback: - Log warning when regex cache mutex is poisoned instead of silently returning None - Add tests verifying `regex` returns false for invalid patterns - Add tests verifying `not_regex` returns true for invalid patterns Co-Authored-By: Claude Opus 4.5 <[email protected]>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Code review follow-ups from PR #36 (the one where I added feature flags support). This PR addresses quick wins that don't require significant refactoring:
.ok()?instead of.unwrap()in regex cache to gracefully handle poisoned mutexesClonetoLocalEvaluator- Enables more flexible usage patternsFlagValue,FeatureFlag,FeatureFlagFilters,Property,LocalEvaluationResponse,Cohort,FlagCache,LocalEvaluationConfig,FlagPoller,AsyncFlagPoller,LocalEvaluator,Error,ClientOptions,FlagDetail,FlagReason,FlagMetadata,FeatureFlagsResponse,hash_key, andget_matching_variantunwrap_orfor defaults, and?propagation