-
Notifications
You must be signed in to change notification settings - Fork 321
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 Response::redirect #435
Conversation
Might be better to have redirect_permanent and redirect_temporary to be explicit? |
Agreed, and that also seems like a distinction the |
@@ -53,6 +53,12 @@ impl Response { | |||
} | |||
} | |||
|
|||
/// Creates a response that represents a temporary redirect to `location`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add an example here?
@jbr I like what you're going for with this PR. I think following @prabirshrestha's suggestion to implement both versions might just be easiest; Tide is still in a place where we can issue breaking changes frequently so if we know which API we want, we can implement it. So my understanding is this would be: fn redirect_permanent;
fn redirect_temporary;
impl Response {
fn redirect_permanent;
fn redirect_temporary;
} I wonder if it would make sense to shorten the suffix here: fn redirect_perm;
fn redirect_temp;
impl Response {
fn redirect_perm;
fn redirect_temp;
} |
3bb1910
to
869be5b
Compare
and use it in the Redirect endpoint
869be5b
to
bf7fbea
Compare
Addressed:
Not [yet] addressed in this PR: Making the same change to the Redirect endpoint / adding a permanent redirect endpoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great; thanks heaps!
The Redirect endpoint is a special case ("always redirect") of the broader need to conditionally redirect based on some logic within a route handler. This PR moves the creation of a temporary redirect into Response and uses that in the Redirect endpoint. Accidentally, this also eliminates a string clone, since set_header only needs a &str.