-
Notifications
You must be signed in to change notification settings - Fork 100
feat(api): add lfs metadata #8
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
Conversation
|
Can we add a test for it ? (Also so I can have a look at what models return that could be interesting in other places to get file sizes without Range:0-0. |
Narsil
left a comment
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.
Quite nice.
I propose a different (simpler imho) internals.
Otherwise looks good. Not sure how cargo fmt didn't pick up those before.
src/api/sync.rs
Outdated
|
|
||
| /// serde_json error | ||
| #[error("Serde json error: {0}")] | ||
| SerdeJson(#[from] serde_json::Error), |
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.
How were we getting away without it before, is it the map(Box?) ?
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.
Response::into_json returns an io::Result<T> while serde_json::from_value returns a Result<T, serde_json::Error>
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.
And reqwest::Response::json returns a Result<T, reqwest::Error>
in favor of returning raw request builder
Narsil
left a comment
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.
LGTM
Full lfs object looks like this :
Needed for the repo scanner