search

owncloud-archive / maps

:globe_with_meridians: Maps app for ownCloud
GNU Affero General Public License v3.0
42 stars 20 forks source link

Cleanup locationcontroller #51

Closed Henni closed 9 years ago

Henni commented 9 years ago

This pull-request removes the deprecated function $this->params() from the locationcontroller and cleans up few other things.

jancborchardt commented 9 years ago

Best for @DJaeger and @brantje to review :)

jancborchardt commented 9 years ago

@Henni can you rebase this?

@DJaeger @brantje @v1r0x @vgezer @povoq can you review? :)

DJaeger commented 9 years ago

Where do you got the information from, that $this->params() is depricated?

As of the docu is to be used: http://api.owncloud.org/classes/OCP.AppFramework.ApiController.html#params

How should the method calls otherwise determine in which order the params have to be applied?

DJaeger commented 9 years ago

The removal of TemplateResponse and addtion of LocationManager is ok.

Henni commented 9 years ago

@DJaeger https://github.com/owncloud/core/blob/master/lib/public/appframework/controller.php#L157 I implemented it like this in the tasks app as well.

LukasReschke commented 9 years ago

How should the method calls otherwise determine in which order the params have to be applied?

That's some magic we do with reflection in core. Let me point you to the source code in core, always good to understand how and why things work like they do :smile:

In https://github.com/owncloud/core/blob/c700f42b68f430e9c89ce97b92ea91323c9f6ed5/lib/private/appframework/http/dispatcher.php we do read the parameters there and do call the function with the right parameters then :)

Henni commented 9 years ago

I removed all the insecure defaults and added simple PHPDocs.

DJaeger commented 9 years ago

Now looks great. :+1: And thanks for the clarification and invitation, @LukasReschke. :-)

LukasReschke commented 9 years ago

YOLO ;-)