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

added UploadResponse support for UploadFileAsync #58

Closed

Conversation

kunalshetye
Copy link
Contributor

The UploadFileAsync method does not return any mediaId of the asset that was uploaded so added UploadFileAsync model to return that information

@coveralls
Copy link

coveralls commented Sep 30, 2021

Coverage Status

Coverage decreased (-0.4%) to 46.613% when pulling a140e4d on kunalshetye:feature/add-uploadfile-response into 7810cb0 on Bynder:master.

@githuib
Copy link
Contributor

githuib commented Sep 30, 2021

Thanks for your pull request. It makes a lot of sense and we actually added a very similar thing to our next big release that is planned to come out soon. The code can be seen here: https://github.com/Bynder/bynder-c-sharp-sdk/tree/FS_upload_dev

On the one hand I think it would be good to have such a change in the current release, but a problem I could see is it will change the return type of AssetService.UploadFileAsync(), therefore breaking the existing API, which could be a problem for some users of the SDK.
The new major release will have several breaking changes, so that's why we did include it there.

For the time being waiting on that major release, what we could consider is adding it as a new method instead of changing the existing one, so it would solve your issue while not breaking the API for other users.

I'll come back at this when we made a decision about this.

@kunalshetye
Copy link
Contributor Author

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

Successfully merging this pull request may close these issues.

None yet

3 participants