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.

Sign up for GitHub

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

Jump to bottom

feat(auth): Add TotpInfo field to UserRecord #573

Merged
merged 14 commits into from
Nov 7, 2023
Merged

feat(auth): Add TotpInfo field to UserRecord #573

merged 14 commits into from
Nov 7, 2023

Conversation

pragatimodi
Copy link
Contributor

@pragatimodi pragatimodi commented Jul 24, 2023 •
edited
Loading

Original PR based off master
This is based off dev branch.

@pragatimodi pragatimodi changed the base branch from master to dev July 24, 2023 20:58
@pragatimodi pragatimodi marked this pull request as ready for review July 24, 2023 21:02
@pragatimodi pragatimodi mentioned this pull request Jul 24, 2023
Closed
lahirumaramba
lahirumaramba reviewed Jul 24, 2023
View reviewed changes
auth/user_mgt.go Outdated Show resolved Hide resolved
Xiaoshouzi-gh
Xiaoshouzi-gh approved these changes Jul 24, 2023
View reviewed changes
Copy link

@Xiaoshouzi-gh Xiaoshouzi-gh left a comment

Choose a reason for hiding this comment

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

Overall LGTM.

Optional - To gradually improve the coding practice, we can start clean up while we touching the existing code. For example, we can start moving repeated string in test cases in constants.

auth/user_mgt.go Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Show resolved Hide resolved
@pragatimodi pragatimodi changed the title feat(auth): Add TotpInfo field to UserRecord feat(auth): Add TotpInfo field to UserRecord Jul 25, 2023
@lahirumaramba lahirumaramba added the release:stage Stage a release candidate label Jul 25, 2023
lahirumaramba
lahirumaramba requested changes Jul 27, 2023
View reviewed changes
Copy link
Member

@lahirumaramba lahirumaramba left a comment

Choose a reason for hiding this comment

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

Looks great overall! Thank you! Added a few more comments.

auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Show resolved Hide resolved
auth/user_mgt_test.go Show resolved Hide resolved
auth/user_mgt_test.go Outdated Show resolved Hide resolved
integration/auth/user_mgt_test.go Show resolved Hide resolved
kevinthecheung
kevinthecheung requested changes Aug 22, 2023
View reviewed changes
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
auth/user_mgt.go Outdated Show resolved Hide resolved
@davidbudnick
Copy link

Good Afternoon Team 👋🏻

Any status updates on getting this MR merged in?

Thanks!

Xiaoshouzi-gh
Xiaoshouzi-gh approved these changes Nov 7, 2023
View reviewed changes
auth/user_mgt.go Show resolved Hide resolved
@pragatimodi pragatimodi merged commit 8355839 into dev Nov 7, 2023
8 checks passed
@pragatimodi pragatimodi deleted the user-record-mfa branch November 7, 2023 21:53
@davidbudnick
Copy link

@pragatimodi 👋🏻 Thanks for pushing the changes to get this merged into the dev branch. Looking through the CONTRIBUTING page I did not see any details about how the release process works for this project.

Do you have any idea on when my team can see it pushed to a new version?

Thanks!

@msidd
Copy link

msidd commented Nov 10, 2023

@pragatimodi Can you please provide ETA on the next release of SDK that has this feature. A critical release is blocked due to this feature missing in the current version of SDK

@pragatimodi
Copy link
Contributor Author

@lahirumaramba Please provide an update on the estimated release for this?

@msidd
Copy link

msidd commented Nov 14, 2023

@lahirumaramba Can you please provide ETA for the next release, as i stated earlier a critical release is blocked due to this. We tried to reach out to GCP premium support if but they pointed us here

@lahirumaramba
Copy link
Member

Thanks for your patience folks! We will include this change in the planned upcoming release this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Xiaoshouzi-gh Xiaoshouzi-gh Xiaoshouzi-gh approved these changes

@kevinthecheung kevinthecheung kevinthecheung approved these changes

@lahirumaramba lahirumaramba lahirumaramba approved these changes

@renkelvin renkelvin Awaiting requested review from renkelvin

@prameshj prameshj Awaiting requested review from prameshj

Assignees
No one assigned
Labels
release:stage Stage a release candidate release-note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@pragatimodi @davidbudnick @msidd @lahirumaramba @kevinthecheung @Xiaoshouzi-gh