Skip to content

Remove PhoneConfirmation namespace#7409

Merged
zachmargolis merged 6 commits intomainfrom
margolis-clean-up-phone-confirmation
Nov 30, 2022
Merged

Remove PhoneConfirmation namespace#7409
zachmargolis merged 6 commits intomainfrom
margolis-clean-up-phone-confirmation

Conversation

@zachmargolis
Copy link
Contributor

The "PhoneConfirmation" namespace had two classes:

  • one single-method class for generating OTPs (inlined)
  • a ConfirmationSession class closely related to IDV, so I moved it there

This is all syntax sugar, small cleanups, if anybody has reasons they want to keep things as-is, I'm happy to close this PR

- Inline logic in ConfirmationSession
…onSession

**Why**: PhoneConfirmation namespace only used in IDV, didn't need to be its own thing
[skip changelog]
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I encountered this recently as well, and based on the commit message of 3a55e8d, I got the impression there'd been some desire to consolidate the handling of OTP confirmation between IdV and MFA. I still think that'd be a nice direction to move, but as things stand, I agree these classes cause me some confusion in how they're used specifically for IdV.

@zachmargolis zachmargolis merged commit 23698df into main Nov 30, 2022
@zachmargolis zachmargolis deleted the margolis-clean-up-phone-confirmation branch November 30, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants