sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Add param and return type hint #6404

Closed VincentLanglet closed 3 years ago

VincentLanglet commented 4 years ago

I didn't find an issue to track the progress. We should add typehint every where on master.

ToDo

napestershine commented 3 years ago

Is this list updated ?

VincentLanglet commented 3 years ago

@napestershine It is now.

There are some WIP PR: Command: https://github.com/sonata-project/SonataAdminBundle/pull/6407 Util: https://github.com/sonata-project/SonataAdminBundle/pull/6366 Admin: https://github.com/sonata-project/SonataAdminBundle/pull/6282 Form + Mapper: https://github.com/sonata-project/SonataAdminBundle/pull/6267

For Datagrid folder, I would say it's ok but there is the following TODO. (Which need a deprecation on 3.x)

    /**
     * TODO: allow only int to be set
     * Sets the maximum number of results per page.
     *
     * @param int|string $max
     */
    public function setMaxPerPage($max): void;

    /**
     * TODO: allow only int to be set
     * Sets the current page.
     *
     * @param int|string $page
     */
    public function setPage($page): void;

DependencyInjection, Event, Guesser, Maker, Menu, Route, Search, Templating, Translator, Twig are not finished

Model is almost done, we just need to discuss about some param like @param mixed $id that we may restrict to int|string.

For Security folder, AdminPermissionMap extends PermissionMapInterface so we can't add param typehint but I think return typehint are allowed.

napestershine commented 3 years ago

Thanks for the update.

eerison commented 3 years ago

Ok @VincentLanglet I'll take a look this one Route.

VincentLanglet commented 3 years ago

@eerison I think Route is started : https://github.com/sonata-project/SonataAdminBundle/pull/6465

eerison commented 3 years ago

@VincentLanglet what didn't start yet?

VincentLanglet commented 3 years ago

@VincentLanglet what didn't start yet?

I would say everything except

eerison commented 3 years ago

I'll see this Util on the weekend :)

@VincentLanglet what didn't start yet?

I would say everything except

* Admin

* Command

* Util

* Form

* Mapper

* Route

* I would not recommend Controller currently.
VincentLanglet commented 3 years ago

I'll see this Util on the weekend :)

You maybe misunderstood. I listed the folder started, and Util is started: https://github.com/sonata-project/SonataAdminBundle/pull/6366

I already listed some WIP PR: https://github.com/sonata-project/SonataAdminBundle/issues/6404#issuecomment-703238750

But you can still work on Util if you want, and start from the @jordisala1991. He opened multiple draft, and already said to me that anybody can take his work and finish it.

tambait commented 3 years ago

I looked through files in Model directory and seems that all files have type hints set (except where method parameters and return value are mixed, those have phpStan descriptions in PHPDoc blocks)

VincentLanglet commented 3 years ago

I looked through files in Model directory and seems that all files have type hints set (except where method parameters and return value are mixed, those have phpStan descriptions in PHPDoc blocks)

Indeed

VincentLanglet commented 3 years ago

BTW @tambait, I think you can finish @jordisala1991 PR if you want ;)

tambait commented 3 years ago

BTW @tambait, I think you can finish @jordisala1991 PR if you want ;)

Yes. I wanted to ask if I could help with some of the PR's which are in progress :)

jordisala1991 commented 3 years ago

Yes, you can take over of my 2 Prs. Thank you for your time!

tambait commented 3 years ago

What's best way to do that? Should I fork your branch and then push commits to it?

jordisala1991 commented 3 years ago

You can try cherry picking my commits and create a new Pr or start over. Otherwise I could be a blocker for this