Skip to content

Conversation

@Potherca
Copy link
Member

@Potherca Potherca commented Jan 11, 2017

Fix for issue #4.

This PR expose a new method run. This can be use from a composer.json file like this:

{
    "name": "potherca/phpcodesniffer-composer-installer-example",
    "description": "Example to demonstrate the working of the dealerdirect/phpcodesniffer-composer-installer package.",
    "require": {
        "dealerdirect/phpcodesniffer-composer-installer": "*",
    },
    "scripts": {
        "run-phpcodesniffer-composer-installer": [
            "Dealerdirect\\Composer\\Plugin\\Installers\\PHPCodeSniffer\\Plugin::run"
        ]
    }
}

Beside this feature I have also done some minor code cleanup and added more message for in verbose mode.

@Potherca
Copy link
Member Author

Potherca commented Feb 3, 2017

@frenck I would suggest merging #6 and #8 first, I'll pick up any merge-conflicts here afterwards and ping you for a review.

@frenck
Copy link
Contributor

frenck commented Feb 5, 2017

@Potherca Will do 👍

@Potherca
Copy link
Member Author

Potherca commented Feb 9, 2017

@frenck Update complete, can be reviewed.

Copy link
Contributor

@frenck frenck left a comment

Choose a reason for hiding this comment

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

Works fine, but missing some code documentation.

$this->init();
}

private function init()
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing documentation block.
Could you please add this?

*/
private $processBuilder;

public static function run(Event $event)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing documentation block.
Could you please add this?

@Potherca
Copy link
Member Author

Docblocks fixed.

@frenck
Copy link
Contributor

frenck commented Feb 14, 2017

Thx!

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

Development

Successfully merging this pull request may close these issues.

2 participants