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

Document the confirm_success_url param for email registration #901

Merged

Conversation

nerfologist
Copy link
Contributor

The confirm_success_url param is required for email registration.
Add mention in the corresponding API endpoint, beside email,
password and password_confirmation.

Also, mention the possibility of specifying default_confirm_success_url
in the initializer as an alternative.

Copy link

@rmcsharry rmcsharry left a comment

Choose a reason for hiding this comment

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

This is sorely needed, thanks!

@lynndylanhurley
Copy link
Owner

@nerfologist - thanks for taking the time to submit this PR. This change looks good, but it looks like there was another edit to this part of the readme that's causing a conflict. Can you please resolve the conflict?

The `confirm_success_url` param is compulsory for registering a new
user. Add mention in the corresponding API endpoint, beside `email`,
`password` and `password_confirmation`.
@nerfologist nerfologist force-pushed the document-confirm_success_url branch from 7eb6cd5 to 3ab8048 Compare October 7, 2017 17:35
@nerfologist
Copy link
Contributor Author

nerfologist commented Oct 7, 2017

Hi, sorry for the delay, I finally found the time to look into it. The 8da9d50 commit by monkypuzzle now mentions the confirm_success_url param. I adjusted the README.md to include some more details which were part of my original PR.

Thanks for this great library 👍

@zachfeldman
Copy link
Contributor

Looking good! Merging. Thanks @nerfologist !

@zachfeldman zachfeldman merged commit 9194a51 into lynndylanhurley:master Oct 7, 2017
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.

4 participants