[identity] dedup Registration and Reserved Username Registration
ClosedPublic
Actions

Authored by varun on Oct 18 2023, 8:21 PM.

Details

Reviewers

bartek
michal
• jon

Commits

rCOMMace0089e8a87: [identity] dedup Registration and Reserved Username Registration

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

Branch

alchemy (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Oct 18 2023, 8:21 PM

Herald added a reviewer: • jon. · View Herald TranscriptOct 18 2023, 8:21 PM

Herald added subscribers: will, tomek, ashoat. · View Herald Transcript

Harbormaster completed remote builds in B23342: Diff 32171.Oct 18 2023, 8:39 PM

varun requested review of this revision.Oct 18 2023, 8:39 PM

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 ↗	(On Diff #32171)	Always prefer `Option<&T>` over `&Option<T>`. Kinda long, but great explanation video here (alternatively a reddit thread) Using slice is a better option too
137–144 ↗	(On Diff #32171)
170 ↗	(On Diff #32171)	Regarding pt 2. in the comment above, I prefer this way here
176 ↗	(On Diff #32171)	These won't be needed if you do `Option<&T>`