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

Reset password options #20

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

Raflos10
Copy link
Contributor

What kind of change does this PR introduce?

Bug fix, feature, docs update, ...

What is the current behavior?

Please link any relevant issues here.

What is the new behavior?

Feel free to include screenshots if it includes visual changes.

Additional context

Add any other context or screenshots.

Copy link
Collaborator

@Proziam Proziam left a comment

Choose a reason for hiding this comment

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

Good catch! The other client libs seem to use both email_redirect_to and redirect_to in their options structs, so there is a lack of consistency that might need some attention. Approved.

https://github.com/supabase/auth-py/blob/bba8033edd5626b8cc18cd1d9f3b09c2c5927e9f/supabase_auth/_sync/gotrue_client.py#L200

@Proziam Proziam merged commit c5e3c26 into supabase-community:main Jan 27, 2025
1 check failed
@Proziam
Copy link
Collaborator

Proziam commented Jan 27, 2025

Looks good, thanks for catching this 💪

@Raflos10
Copy link
Contributor Author

Good catch! The other client libs seem to use both email_redirect_to and redirect_to in their options structs, so there is a lack of consistency that might need some attention. Approved.

https://github.com/supabase/auth-py/blob/bba8033edd5626b8cc18cd1d9f3b09c2c5927e9f/supabase_auth/_sync/gotrue_client.py#L200

Yeah, that's strange. It seems like on the js client it only uses "redirect_to" so maybe it's safe to stick to that one?
Thanks for merging!

@Proziam
Copy link
Collaborator

Proziam commented Jan 27, 2025

The JS library is the 'main' one so it makes sense to unify things around that one I believe. I will have to look out for more inconsistencies like this, so I'm grateful you found it

@Raflos10
Copy link
Contributor Author

No problem. I appreciate this repo!

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.

2 participants