pixie icon indicating copy to clipboard operation
pixie copied to clipboard

Fix PHP Doc Blocks

Open DarkMukke opened this issue 7 years ago • 14 comments

There seem to be many PHP Doc block incorrectly defined.

Eg :

class QueryBuilderHandler
{
   ...
    /**
     * @var null|PDOStatement
     */
    protected $pdoStatement = null;

    ...
    /**
     * @param $tables Single table or multiple tables as an array or as
     *                multiple parameters
     *
     * @return static
     */
    public function table($tables)
    {

needs to be

class QueryBuilderHandler
{
   ...
    /**
     * @var null|\PDOStatement
     */
    protected $pdoStatement = null;

    ...
    /**
     * @param string|array $tables Single table or multiple tables as an array or as
     *                multiple parameters
     *
     * @return static
     */
    public function table($tables)
    {

on a side note, in php 5.6 we should use splat vs func_get_args

DarkMukke avatar Sep 27 '16 20:09 DarkMukke

IF you want i can create a PR for that as I already fixed it locally

DarkMukke avatar Sep 27 '16 20:09 DarkMukke

@DarkMukke go ahead. Thank you.

TCB13 avatar Oct 17 '17 12:10 TCB13

This make my #167 a duplicate. You've the code, just PR.

TCB13 avatar Oct 17 '17 12:10 TCB13

The PR is not perfect though, it fails a test and I did this too long ago to remember why. I had to re-fork and make some changes so it took me a bit to get around to do it.

DarkMukke avatar Oct 18 '17 12:10 DarkMukke

@DarkMukke I noticed there are other additional changes not related to the DocBlocks. What about just committing DocBlock changes right now and ignore everything else? 👍

TCB13 avatar Oct 18 '17 12:10 TCB13

Right, that's how I had it in my local, Will make a small PR for just dock blocks

DarkMukke avatar Oct 19 '17 10:10 DarkMukke

@DarkMukke any news on the smaller just DocBlock PR?

TCB13 avatar Nov 24 '17 12:11 TCB13

@TCB13 been on holiday (off-line), it's on my todo-list

DarkMukke avatar Nov 28 '17 10:11 DarkMukke

Any updates on this?

allejo avatar Feb 08 '18 09:02 allejo

Oh woops, i have the commit, let make it a PR

DarkMukke avatar Feb 08 '18 10:02 DarkMukke

this is fixed by #183 https://github.com/usmanhalalit/pixie/pull/183/files#diff-4d19ae281a7c0cb6c93d185dbb1108ffR447

vikkio88 avatar Apr 11 '18 14:04 vikkio88

@DarkMukke you using phpstorm arent you? LoL I found this annoying to and fixed it while fixing another bug

vikkio88 avatar Apr 11 '18 14:04 vikkio88

Yeah @vikkio88 it is true. @DarkMukke can you check if the changes by @vikkio88 at 220b920904b9f60a7d61eba9554d936f4fda7977 completely fix this? In my opinion they do.

TCB13 avatar Apr 11 '18 14:04 TCB13

I tested in my local env and they did. but is good to have another pair of eyes on it

vikkio88 avatar Apr 11 '18 14:04 vikkio88