New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean up FieldValueTests #446
Conversation
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.
Nice. I think there's still a bit of fuzziness in that most of the (now-)UserDataConverter tests also test FieldValue.value() (i.e. the full round-trip from value => FieldValue => value) which isn't done by UserDataConverter but I don't think it's worth trying to split that apart.
@@ -37,7 +38,7 @@ | |||
@PublicApi | |||
public final class Timestamp implements Comparable<Timestamp>, Parcelable { | |||
|
|||
@PublicApi | |||
@PublicApi @NonNull |
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.
Newline?
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 had a newline here but googleJavaFormat
eated it.
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.
How rude!
/retest |
1 similar comment
/retest |
Reorganize to make things clearer:
UserDataConverter
into aUserDataConverterTest
.FieldValue
intoTimestampTest
Also fix nullability annotations on
Timestamp
.