Skip to content

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

Merged
merged 6 commits into from
Jun 24, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,7 @@ logs

# ignore test results
phpunit-test-results

*.log

coverage.xml
5 changes: 3 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@ php:
# - nightly
before_install:
- nvm install 6.11
install:
install:
- composer install
- npm install
before_script:
- npm start 1>&2
- sleep 3
- npm run lint
script:
- ./vendor/bin/phpunit --coverage-clover=coverage.xml
- npm run test:coverage

after_success:
- bash <(curl -s https://codecov.io/bash)
12 changes: 10 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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`
Copy link
Contributor

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.


Make sure your code is linted with phpcs (PSR-2 Coding Style)
Copy link
Contributor

Choose a reason for hiding this comment

The 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...
Make sure your code is linted with phpcs (PSR-2 Coding Style)

Copy link
Contributor

Choose a reason for hiding this comment

The 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:

You may also run tests directly using phpunit as follows:


`npm run lint`
Copy link
Contributor

Choose a reason for hiding this comment

The 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 npm test above and npm run lint:fix below.

I like the ability to lint via npm though, this is a lot more concise than calling phpunit directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

nice catch


You can automatically fix lint errors with phpcbf
Copy link
Contributor

Choose a reason for hiding this comment

The 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`
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
},
"require-dev": {
"phpunit/phpunit": "~4.0",
"squizlabs/php_codesniffer": "~1.5",
"squizlabs/php_codesniffer": "^3.0.1",
"phpdocumentor/phpdocumentor": "~2.5"
},
"autoload": {
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
{
"name": "parse-php-sdk",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1",
"test": "./vendor/bin/phpunit",
"test:coverage": "./vendor/bin/phpunit --coverage-clover=coverage.xml",
"lint": "./vendor/bin/phpcs --standard=./phpcs.xml.dist ./src/Parse ./tests/Parse",
"lint:fix": "./vendor/bin/phpcbf --standard=./phpcs.xml.dist ./src/Parse ./tests/Parse",
"start" : "./node_modules/parse-server-test/run-server",
"stop" : "./node_modules/parse-server-test/stop-server"
},
Expand Down
14 changes: 14 additions & 0 deletions phpcs.xml.dist
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>
35 changes: 9 additions & 26 deletions src/Parse/HttpClients/ParseCurl.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace Parse\HttpClients;


use Parse\ParseException;

/**
Expand All @@ -27,11 +26,9 @@ class ParseCurl
*/
public function init()
{
if($this->curl === null) {
if ($this->curl === null) {
$this->curl = curl_init();

}

}

/**
Expand All @@ -42,9 +39,8 @@ public function init()
*/
public function exec()
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

return curl_exec($this->curl);
Expand All @@ -59,13 +55,11 @@ public function exec()
*/
public function setOption($option, $value)
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

curl_setopt($this->curl, $option, $value);

}

/**
Expand All @@ -76,13 +70,11 @@ public function setOption($option, $value)
*/
public function setOptionsArray($options)
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

curl_setopt_array($this->curl, $options);

}

/**
Expand All @@ -94,13 +86,11 @@ public function setOptionsArray($options)
*/
public function getInfo($info)
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

return curl_getinfo($this->curl, $info);

}

/**
Expand All @@ -111,13 +101,11 @@ public function getInfo($info)
*/
public function getError()
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

return curl_error($this->curl);

}

/**
Expand All @@ -128,31 +116,26 @@ public function getError()
*/
public function getErrorCode()
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

return curl_errno($this->curl);

}

/**
* Closed our curl handle and disposes of it
*/
public function close()
{
if(!isset($this->curl)) {
if (!isset($this->curl)) {
throw new ParseException('You must call ParseCurl::init first');

}

// close our handle
curl_close($this->curl);

// unset our curl handle
$this->curl = null;

}

}
}
Loading