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

Require PHP 7.1, add phpstan, Doctrine coding standards, add scrutini… #38

Merged
merged 1 commit into from
May 29, 2018

Conversation

jwage
Copy link
Owner

@jwage jwage commented May 23, 2018

…zer and update travis ci.

@jwage jwage added this to the 0.0.5 milestone May 23, 2018
@jwage jwage force-pushed the code-cleanup branch 3 times, most recently from 9ca129c to b08f65e Compare May 23, 2018 20:01
protected $enclosure = '"';

/**
* @param string[] $initialHeaders
Copy link
Contributor

@peter279k peter279k May 28, 2018

Choose a reason for hiding this comment

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

The parameters are $path, $modeand $isNeedBOM.
They're needed to add the @param in that doc block.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I am adopting the https://github.com/doctrine/coding-standard in this project and it requires only having doc blocks for parameters that can't be hinted by the PHP language itself.

/**
* @param $path
* @param string $mode
* @param bool $headersInFirstRow
Copy link
Contributor

@peter279k peter279k May 28, 2018

Choose a reason for hiding this comment

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

Can you explain why remove these doc blocks?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Because the type hints in the PHP language define everything needed so the doc blocks are not required.

}

/**
* @param $lineNumber zero-based index
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same question as the previous comment.

@@ -1,3 +1,5 @@
<?php

declare(strict_types=1);
Copy link
Contributor

@peter279k peter279k May 28, 2018

Choose a reason for hiding this comment

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

The declare(strict_types=1); is added in the every related Tests methods currently.
This bootstrap file seems to be useless and it should change this approach to adding the vendor/autload.php in bootstrap attribute for the phpunit tag in phpunit.xml.dist file.

Here is the example:

<phpunit bootstrap="./vendor/autoload.php" colors="true">

Copy link
Owner Author

Choose a reason for hiding this comment

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

Agree. I will make that change.

@jwage jwage merged commit dc488d1 into master May 29, 2018
@jwage jwage deleted the code-cleanup branch May 29, 2018 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants