-
-
Notifications
You must be signed in to change notification settings - Fork 342
Update PHP_CodeSniffer #328
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
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,7 @@ logs | |
|
||
# ignore test results | ||
phpunit-test-results | ||
|
||
*.log | ||
|
||
coverage.xml |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,7 @@ To setup the Test Parse Server: | |
* Run `npm install` from the project root to download the server and it's dependencies. | ||
* When you're ready to run tests use `npm start` from the project root to boot up the test server. | ||
|
||
The embedded test server utilizes this [parse server test] project. | ||
The embedded test server utilizes this [parse server test] project. | ||
It's setup with the appropriate configuration to run the php sdk test suite. | ||
Additionally it handles setting up mongodb for the server. | ||
|
||
|
@@ -63,7 +63,15 @@ Alternately you can configure a compatible test server as follows: | |
|
||
You should now be able to execute the tests, from project root folder: | ||
|
||
./vendor/bin/phpunit | ||
`npm test` | ||
|
||
Make sure your code is linted with phpcs (PSR-2 Coding Style) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I figure just so people aren't wondering about why we would enforce PSR-2 we should make this a link. You can use http://www.php-fig.org/psr/psr-2/, which is the style guide published by the framework interop group. Making it... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And if we can add a colon after this statement, same as the line above it:
|
||
|
||
`npm run lint` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you take a look here your code block comment is rendering with the grave ticks still. Same for I like the ability to lint via npm though, this is a lot more concise than calling phpunit directly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice catch |
||
|
||
You can automatically fix lint errors with phpcbf | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And if we can add a colon here at the end of this statement too that would be fantastic. |
||
|
||
`npm run lint:fix` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, nice to have an alias in a sense. I'm guessing all these changes are from the auto linting? We haven't been enforcing strong linting for some time it would appear. I'll just glance over all these to make sure they're acceptable; I'm expecting they are however. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah auto linting is my friend on this one |
||
|
||
The test suite is setup for code coverage if you have [XDebug] installed and setup. | ||
Coverage is outputted as text and as html in the phpunit-test-results/ directory within the project root. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
<?xml version="1.0" encoding="UTF-8"?> | ||
<ruleset name="phpcsignore"> | ||
<description>PHPCS Ignore Rule</description> | ||
<rule ref="PSR2" /> | ||
<rule ref="PSR1.Files.SideEffects.FoundWithSymbols"> | ||
<severity>0</severity> | ||
</rule> | ||
<rule ref="PSR2.Methods.MethodDeclaration"> | ||
<severity>0</severity> | ||
</rule> | ||
<rule ref="PSR2.Classes.PropertyDeclaration.Underscore"> | ||
<severity>0</severity> | ||
</rule> | ||
</ruleset> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I would like to retain both ways of running tests in the documentation. In some cases local users may want to use an alternate version of php besides the default to run phpunit under. Via
npm test
there's really no way to do this.You can add it after the npm marks here, since those are a bit more manageable and most will be ready to go that route (I would imagine at least). Something like
You may also run tests directly using phpunit as follows
. As for the lint and lint:fix don't worry about adding their equivalents, the npm ones will be fine and are (personally) easier to remember than their equivalents.