Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

UploadFile should probably get a reader as an argument #1317

Closed
mitar opened this issue Dec 15, 2021 · 3 comments
Closed

UploadFile should probably get a reader as an argument #1317

mitar opened this issue Dec 15, 2021 · 3 comments

Comments

@mitar
Copy link
Contributor

mitar commented Dec 15, 2021

I find UploadFile function signature strange:

func (s *ProjectsService) UploadFile(pid interface{}, file string, options ...RequestOptionFunc) (*ProjectFile, *Response, error)

I think it would be better to have:

func (s *ProjectsService) UploadFile(pid interface{}, r io.Reader, filename string, options ...RequestOptionFunc) (*ProjectFile, *Response, error)

In this way you can upload generated file without having to store it to a file first.

This would be a breaking change but I think it would be worth it.

@svanharmelen
Copy link
Member

As just mentioned in the other thread, I try to follow the GitLab API and examples as close as possible. If you look at https://docs.gitlab.com/ce/api/projects.html#upload-a-file you can see the example there matches this use case exactly.

Yet I can see that for some use cases it would be nice (and potentially easier) to be able to pass a reader instead, so I'm open to change this one to make it more usable.

@mitar
Copy link
Contributor Author

mitar commented Dec 15, 2021

Awesome. I made #1318 in this spirit, if you review that, I can extract shared code out and use it for both API calls, I think.

I agree that following examples is useful, but we also want idiomatic Go interfaces in Go package. :-)

@svanharmelen
Copy link
Member

Fixed by #1318

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants