This enum derives Responder, so it can be used as route result.
We also implement From, so it can be converted

This pr addresses #613

Codecov Report

Merging #614 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master     #614   +/-   ##
=======================================
  Coverage   35.31%   35.31%           
=======================================
  Files          68       68           
  Lines        7907     7907           
  Branches     1893     1893           
=======================================
  Hits         2792     2792           
  Misses       4345     4345           
  Partials      770      770

I’ve marked this pull request now ready for review.
even though there’s at least one function left that could be improved (in readability, or at least refractored to use this enum)
but i don’t think we can make further strides without improving our ErrorPage

rfwatson (Migrated from github.com) reviewed 1 year ago

src/routes/blogs.rs

Plume_migration_agent commented 1 year ago

So if I read this correctly, the Rust compiler infers the type from the return type of the function, allowing into() to function as expected?

Even though I should probably expect that from Rust, that’s still pretty neat.

trinity-1686a reviewed 1 year ago

👍

src/routes/session.rs

trinity-1686a commented 1 year ago

Isn’t it a bit strange that when redirecting to invalid uri, it gives an error? It would probably make more sens that the chain on line 100 does the parsing to, and default to parse("/").unwrap(). Also it should probably be a Origin to block redirection to other domain
(this is also true for previous code, it’s pretty old, I’m pretty sure we did not do code review last September)

igalic (Migrated from github.com) reviewed 1 year ago

src/routes/session.rs

Plume_migration_agent commented 1 year ago

do you have a suggestion for improvement?

trinity-1686a reviewed 1 year ago

src/routes/session.rs

trinity-1686a commented 1 year ago

I do but I don’t know how to explain it with words, so I’ll probably do it myself during the week end

trinity-1686a approved these changes 1 year ago

igalic (Migrated from github.com) reviewed 1 year ago

src/routes/session.rs

Plume_migration_agent commented 1 year ago

well, I’ll be happy to review it then, and help with words where necessary

i used to be quite good with words, now i’m procrastinating on writing words, by working on a blogging platform.

Reviewers

trinity-1686a approved these changes 1 year ago

The pull request has been merged as 3d27e283ad.

trinity-1686a

A: Backend

S: Ready for review

No Milestone

No Assignees

2 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

#614 add enum containing all successful route returns

Codecov Report

Reviewers