Skip to content
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

Adds wrapper for blob content #158

Closed
wants to merge 10 commits into from

Conversation

4 participants
@richmahn
Copy link

commented Apr 1, 2019

This change is made for issue https://github.com/go-gitea/gitea/issue/4762 in relation to review of go-gitea/gitea#6314, making the Blob data be encoded on Marshalling.

Show resolved Hide resolved gitea/git_blob.go Outdated

@GiteaBot GiteaBot added the lgtm/need 2 label Apr 1, 2019

@zeripath
Copy link

left a comment

Hmm. You're probably gonna hate me but I don't think we need this after all.

It doesn't really fix the issue with blobs being very large as marshalJson will still put the whole thing in memory.

There's no requirement for us to ever actually completely instantiate a BlobResponse on server we just need to emit it.

So we don't need to do this wrapper. (At least not on the server - we should just change the server to emit the JSON - although I'll comment on the pr about this.) At client level later on we might also want to do some clever stuff but this API should still be correct.

@richmahn

This comment has been minimized.

Copy link
Author

commented Apr 9, 2019

Closed per review/request

@richmahn richmahn closed this Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.