Refactor #6
Refactor #6
Conversation
| #[derive(Debug)] | ||
| pub enum UrlParseError { | ||
| NoPath, | ||
| NotHttps, | ||
| Parser(url::ParseError), | ||
| } |
There was a problem hiding this comment.
Do you really need this, or does url's ParseError cover everything? It seems to me that it does: https://docs.rs/url/latest/url/enum.ParseError.html#variants
| if uri.scheme() != "https" { | ||
| return Err(UrlParseError::NotHttps); | ||
| } |
There was a problem hiding this comment.
What's the reasoning behind mandating https? If Firebase has a problem with http, it should probably be reported from their side, not manually from the library.
| use std::fmt::{Display, Formatter}; | ||
|
|
||
| #[derive(Debug)] | ||
| pub enum UrlParseError { |
There was a problem hiding this comment.
You should create a type alias to Result<T, UrlParseError> and use it instead, e.g. in src/utils.rs' check_uri function.
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub enum RequestError { |
There was a problem hiding this comment.
The same comment regarding a custom Result type goes for here.
| /* | ||
| Firebase REST API | ||
| */ |
There was a problem hiding this comment.
This top-level crate documentation was a great idea. Maybe not in this PR, but eventually I highly recommend writing doc comments for everything, including top-level module documentation.
No description provided.