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

Move Tags/Controller over to a real Controller #958

Closed
wants to merge 2 commits into from
Closed

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Aug 19, 2016

Follow up of #947

This moves the tags 'controller' over to a real controller and leverages all the DI the AppFramework gives us.

There were no tests. And are no tests so far. If somebody desperatly wants them feel free to add.

CC: @nickvergessen @MorrisJobke @LukasReschke @icewind1991

P.s. I still vote to kill this off in time.

@rullzer rullzer added the 3. to review Waiting for reviews label Aug 19, 2016
@rullzer rullzer added this to the Nextcloud 11.0 milestone Aug 19, 2016
@mention-bot
Copy link

@rullzer, thanks for your PR! By analyzing the annotation information on this pull request, we identified @tanghus, @nickvergessen and @MorrisJobke to be potential reviewers

@rullzer
Copy link
Member Author

rullzer commented Aug 19, 2016

Lets deprecate this internal route (How do we signal this?)
And remove this in NC12. As I dont' believe it is used anywhere.

@nickvergessen
Copy link
Member

Announce in the dev forum

@@ -0,0 +1,232 @@
<?php
/**
*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing @copyright header?

@LukasReschke
Copy link
Member

LGTM

@rullzer
Copy link
Member Author

rullzer commented Aug 24, 2016

So it seems in the old code the route

/tags/files/ids

Throws an exception.
It becomes less and less likely any of this stuff is used.

I vote again to remove.

@rullzer rullzer closed this Aug 26, 2016
@rullzer rullzer deleted the tagController branch August 26, 2016 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants