-
-
Notifications
You must be signed in to change notification settings - Fork 70
Migrate to null-safety #11
Conversation
@phamhieu |
@phamhieu |
@phamhieu |
lib/src/auth_user.dart
Outdated
required String email, | ||
required String createdAt, | ||
required String confirmedAt, | ||
required String lastSignInAt, |
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.
confirmedAt and lastSignInAt are nullable. So we don't need to required them.
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.
Done
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.
actually, i'm not sure about the use-case for AuthUser
#6
for now, just follow exactly like User
constructor on gotrue-dart
.
User(
{required this.id,
required Map<String, dynamic>? appMetadata,
required Map<String, dynamic>? userMetadata,
required this.aud,
required this.email,
required this.createdAt,
this.confirmedAt,
this.lastSignInAt,
required this.role,
required this.updatedAt})
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.
what do you think? @TimWhiting
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.
Yeah, I think the gotrue class should just be exported directly, rather than wrapping it. I'm not sure this class will be useful, and if it is, it can be added in later.
just 1 change left. otherwise lgtm 👍 |
What kind of change does this PR introduce?
Migrates to null safety
What is the current behavior?
Using null unsafe dart
What is the new behavior?
Using null safe dart
Additional context
Addresses #10
@phamhieu