Page MenuHomePhabricator

[identity] dedup Registration and Reserved Username Registration
ClosedPublic

Authored by varun on Oct 18 2023, 8:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 19, 4:20 AM
Unknown Object (File)
Sat, Oct 19, 4:20 AM
Unknown Object (File)
Sat, Oct 19, 4:20 AM
Unknown Object (File)
Sat, Oct 19, 4:18 AM
Unknown Object (File)
Sat, Oct 19, 4:02 AM
Unknown Object (File)
Sep 18 2024, 9:12 PM
Unknown Object (File)
Sep 18 2024, 9:11 PM
Unknown Object (File)
Sep 18 2024, 9:19 AM
Subscribers

Details

Summary

a refactor to reduce some code duplication

Depends on D9526

Test Plan

successfully registered a new user with the normal registration workflow and registered an existing user with the reserved registration workflow. everything still works

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Oct 18 2023, 8:39 PM
bartek requested changes to this revision.Oct 20 2023, 2:51 AM

Looks much better!
Please consider my suggestion about Option<&T> over &Option<T> and we're good to go 💪

services/identity/src/grpc_utils.rs
133–134
  1. Always prefer Option<&T> over &Option<T>. Kinda long, but great explanation video here (alternatively a reddit thread)
  2. Using slice is a better option too
137–144
170

Regarding pt 2. in the comment above, I prefer this way here

176

These won't be needed if you do Option<&T>

This revision now requires changes to proceed.Oct 20 2023, 2:51 AM
This revision is now accepted and ready to land.Oct 20 2023, 7:49 AM