Skip to content
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

fix: move MFA sub types to internal file #964

Merged
merged 1 commit into from
Oct 8, 2024
Merged

fix: move MFA sub types to internal file #964

merged 1 commit into from
Oct 8, 2024

Conversation

J0
Copy link
Contributor

@J0 J0 commented Sep 27, 2024

What kind of change does this PR introduce?

Follow up to #956, hides the MFA sub-types (e.g. MFA Phone, MFA TOTP) needed to fix type errors before we release to public so that we can remove them as and when we like. The sub-types are now stored in an internal-types file which, unlike the types file is not exposed to public.

The types mentioned here have not been released yet so fine to hide them

/** Type of MFA factor. */
type: 'phone'

/** Friendly name of the factor, useful for distinguishing between factors **/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only export the types file so this shouldn't get exposed

Copy link
Contributor

@aloisklink aloisklink left a comment

Choose a reason for hiding this comment

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

I've tested this and it seems to work fine!

This is exactly what I was thinking of in #956 (comment) 🚀

Depending on your release/coding style, consider making this a refactor: commit/PR instead. It looks like you're automatically generating release notes from the commits, and so you might not want this to show up in the release notes, since it only "fixes" unreleased APIs!

(this is probably my main issue with conventional commits and automated changelogs, it's a bit difficult to mark a commit that changes unreleased behavior).

if (
error?.code === 'identity_already_exists'
) {
if (error?.code === 'identity_already_exists') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

prettier

@J0 J0 merged commit 4b7455c into master Oct 8, 2024
6 checks passed
@J0 J0 deleted the j0/hide_mfa_types branch October 8, 2024 11:12
kangmingtay pushed a commit that referenced this pull request Oct 15, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.65.1](v2.65.0...v2.65.1)
(2024-10-14)


### Bug Fixes

* Call `SIGNED_OUT` event where session is removed
([#854](#854))
([436fd9f](436fd9f))
* improve `mfa.enroll` return types
([#956](#956))
([8a1ec06](8a1ec06))
* move MFA sub types to internal file
([#964](#964))
([4b7455c](4b7455c))
* remove phone mfa deletion, match on error codes
([#963](#963))
([ef3911c](ef3911c))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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