8
16
Fork 22

add enum containing all successful route returns #614

Zusammengeführt
igalic hat 10 Commits von refactor/route-result nach master vor 5 Jahren zusammengeführt
igalic hat vor 5 Jahren kommentiert (Migriert von github.com)

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

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[bot] hat vor 5 Jahren kommentiert (Migriert von github.com)

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
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/614?src=pr&el=h1) Report > Merging [#614](https://codecov.io/gh/Plume-org/Plume/pull/614?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/4b205fa995a6935a2512a9725d84635e889db48e?src=pr&el=desc) will **not change** coverage. > The diff coverage is `0%`. ```diff @@ 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 ```
igalic hat vor 5 Jahren kommentiert (Migriert von github.com)

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

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 (Migriert von github.com) hat vor 5 Jahren überprüft
rfwatson (Migriert von github.com) hat vor 5 Jahren kommentiert

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.

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 hat vor 5 Jahren überprüft
trinity-1686a hat einen Kommentar hinterlassen
Besitzer

👍

:+1:
trinity-1686a hat vor 5 Jahren kommentiert
Besitzer

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)

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](https://api.rocket.rs/v0.4/rocket/http/uri/struct.Origin.html) 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 (Migriert von github.com) hat vor 5 Jahren überprüft
igalic (Migriert von github.com) hat vor 5 Jahren kommentiert

do you have a suggestion for improvement?

do you have a suggestion for improvement?
trinity-1686a hat vor 5 Jahren überprüft
trinity-1686a hat vor 5 Jahren kommentiert
Besitzer

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

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 hat die Änderungen vor 5 Jahren genehmigt
igalic (Migriert von github.com) hat vor 5 Jahren überprüft
igalic (Migriert von github.com) hat vor 5 Jahren kommentiert

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.

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.

Reviewer

trinity-1686a hat die Änderungen vor 5 Jahren genehmigt
Der Pull Request wurde als 3d27e283ad gemergt.

Schritt 1:

Wechsle auf einen neuen Branch in deinem lokalen Repository und teste die Änderungen.
git checkout -b refactor/route-result master
git pull origin refactor/route-result

Schritt 2:

Führe die Änderungen zusammen und aktualisiere den Stand online auf Forgejo.
git checkout master
git merge --no-ff refactor/route-result
git push origin master
Anmelden, um an der Diskussion teilzunehmen.
Keine Reviewer
Kein Meilenstein
Niemand zuständig
2 Beteiligte
Nachrichten
Fällig am
Das Fälligkeitsdatum ist ungültig oder außerhalb des zulässigen Bereichs. Bitte verwende das Format „jjjj-mm-tt“.

Kein Fälligkeitsdatum gesetzt.

Abhängigkeiten

Keine Abhängigkeiten gesetzt.

Referenz: Plume/Plume#614
Laden…
Hier gibt es bis jetzt noch keinen Inhalt.