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

Add Support for FCMv1 Server Credentials in MultiFactorPush Sub-Manager of GuardianManager #512

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

chrisnellis
Copy link
Contributor

@chrisnellis chrisnellis commented Feb 19, 2025

🔧 Changes

📚 References

🔬 Testing

  • Ran initial tests to verify changes
  • Added Test Recordings

📝 Checklist

  • All new, modified, or fixed functionality is covered by tests
  • Documentation has been updated for all new/changed functionality

First-time contributor—happy to make any modifications as needed! 😊

@chrisnellis chrisnellis requested a review from a team as a code owner February 19, 2025 02:52
@developerkunal
Copy link
Contributor

Hi @chrisnellis,

I hope you're doing well!

Thank you for opening the PR. Could you please sign your commits as well? We have a new policy requiring this, and I’ll be updating the contribution guide accordingly.

I'll review the PR, and if needed, I can make changes as well.

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.61%. Comparing base (0a40d29) to head (c7921e0).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #512   +/-   ##
=======================================
  Coverage   95.61%   95.61%           
=======================================
  Files          57       57           
  Lines       11267    11276    +9     
=======================================
+ Hits        10773    10782    +9     
  Misses        373      373           
  Partials      121      121           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@developerkunal developerkunal changed the title Add support for FCM Server Credentials Add Support for FCMv1 Server Credentials in MultiFactorPush Manager Feb 19, 2025
@developerkunal
Copy link
Contributor

Hi @chrisnellis,

I've added the test recordings and verified them. I also noticed the getters were missing—we use the make generate command to generate getters for all structs, so I’ve taken care of that as well.

The only thing left is signing the commits. Once that’s done, we should be good to go!

Thank you for your contribution. 🚀

@developerkunal developerkunal changed the title Add Support for FCMv1 Server Credentials in MultiFactorPush Manager Add Support for FCMv1 Server Credentials in MultiFactorPush Sub-Manager of GuardianManager Feb 19, 2025
@chrisnellis
Copy link
Contributor Author

Thanks, @developerkunal

I signed and rebased the commits and re-pushed. Please review at your leisure

Copy link
Contributor

@developerkunal developerkunal left a comment

Choose a reason for hiding this comment

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

LGTM

@ramya18101 ramya18101 merged commit 32ce750 into auth0:main Feb 20, 2025
6 checks passed
@chrisnellis
Copy link
Contributor Author

chrisnellis commented Feb 20, 2025

Very good. Thank you for the help getting this merged!

I eagerly await the next release so I can start to add Terraform support for this field

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.

Add FCMv1 Option to the Go SDK
4 participants