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

OCSResponse cleanup #1056

Closed
rullzer opened this issue Aug 25, 2016 · 4 comments
Closed

OCSResponse cleanup #1056

rullzer opened this issue Aug 25, 2016 · 4 comments
Assignees

Comments

@rullzer
Copy link
Member

rullzer commented Aug 25, 2016

Currently we have the OCSResponse in OCP https://github.com/nextcloud/server/blob/master/lib/public/AppFramework/Http/OCSResponse.php

However I question the use of this. Since people writing OCS code will use the OCSController which takes a DataResponse: https://github.com/nextcloud/server/blob/master/lib/public/AppFramework/Http/OCSResponse.php

On top of that the first OCSResponse parameter is the format string https://github.com/nextcloud/server/blob/master/lib/public/AppFramework/Http/OCSResponse.php#L57 which is weird since it is the job of the AppFramework/Controller to set this properly. When I'm writing my endpoint I don't want to care. I just want to return data.

So what I propose:

  1. Deprecate OCSResponse in OCP
  2. Create an internal OCSResponse type that does the correct magic

Good/bad idea? Comments? @BernhardPosselt @LukasReschke @nickvergessen @MorrisJobke

@rullzer
Copy link
Member Author

rullzer commented Sep 4, 2016

I assume your silence means go for it ;)
I'll tackle this soonish

@rullzer rullzer self-assigned this Sep 4, 2016
@nickvergessen
Copy link
Member

👍

@nickvergessen
Copy link
Member

But I'd just do 1. now, and waste no more time with it, aka we don't need an OCSResponse you said, so no need to write internal magic code for it?

@rullzer
Copy link
Member Author

rullzer commented Sep 5, 2016

Well we use it in the middleware etc. But we can clean that up of course as well.

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

No branches or pull requests

2 participants