fix: make Status ToString() methods format consistent #247
Open
HarleyRossetto wants to merge 1 commit intolaunchdarkly:mainfrom
Open
fix: make Status ToString() methods format consistent #247HarleyRossetto wants to merge 1 commit intolaunchdarkly:mainfrom
HarleyRossetto wants to merge 1 commit intolaunchdarkly:mainfrom
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit ad283f7. Configure here.
…gSegmentStoreStatus ToString()
The BigSegmentStoreStatus struct returns data in the format
(Available={0},Stale{1)} injecting the fields in allowing the type to be
easily stringified.
DataSourceStatus and DataStoreStatus only output a string of the
internal fields without labels, making decoding the results difficult
without understanding the datils of these types.
This change adds in the labeling of fields so they can be clearly
understood when using the default ToString() implementation.
BigSegmentStoreStatus has also been updated to include its type name
like the other two status providers. Now all follow the pattern
Type(Field=Value,Field2=Value...)
ad283f7 to
888887f
Compare
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
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.

Requirements
Related issues
Provide links to any issues in this repository or elsewhere relating to this pull request.N/A
Describe the solution you've provided
The different
Statusstructs that expose internal state of the client have different formatted when string-ified. If the state is logged or otherwise serialised as a string it is not possible to determine what the values mean without understanding the underlying types. This change aligns 3 statusToString()methods to follow a consistent format, including labelling for the different fields exposed.The BigSegmentStoreStatus struct returns data in the format
(Available={0},Stale{1)}injecting the fields in allowing the type to be easily string-ified.DataSourceStatus and DataStoreStatus only output a string of the internal fields without labels, making decoding the results difficult without understanding the details of these types.
e.g.
DataStoreStatus(True,False)DataSourceStatus(Initializing,4/10/2026 10:42:08 PM,)or
DataSourceStatus(Initializing,4/10/2026 10:42:08 PM,UNKNOWN(Connection refused (localhost:5279))@4/10/2026 10:42:18 PM)when errors are present.DataSourceStatusis reasonably understandable as is, howeverDataStoreStatusis not.This change adds in the labelling of fields so they can be clearly understood when using the default ToString() implementation.
BigSegmentStoreStatus has also been updated to include its type name like the other two status providers. Now all follow the pattern set by the other two providers:
Type(Field=Value,Field2=Value...).(Available=False,Stale=False)to
BigSegmentStatus(Available=False,Stale=False)Also include minor changes to
ErrorInfo'sToString()method to utilise character append overloads for single characters rather than string variation as it is a cheaper call.Describe alternatives you've considered
I considered applying this logic to
ErrorInfo'sToString(), however the output of that is fairly clear and can be understood without additional labels.Additional context
Test coverage was ignored as no existing tests exist for these methods, and they are not utilised by the SDK itself.
Note
Low Risk
Low risk formatting-only changes, but could affect consumers that parse log/diagnostic strings from
ToString()output.Overview
Standardizes the string representation of
BigSegmentStoreStatus,DataSourceStatus, andDataStoreStatussoToString()consistently emitsType(Field=value,...)with labeled fields.Also makes a small micro-optimization in
DataSourceStatus.ErrorInfo.ToString()by usingStringBuilder.Append(char)overloads for single-character tokens.Reviewed by Cursor Bugbot for commit 888887f. Bugbot is set up for automated code reviews on this repo. Configure here.