Update PHP_CodeSniffer #328

dplewis · 2017-06-23T16:48:42Z

I've added a few commands to the package.json file.

npm test - shortcut for tests
npm run test:coverage - for travis
npm run lint - Lints src and test folders
npm run lint:fix - Automatically Lints files

I've also forced travis to lint files.

CONTIBUTING.md has been updated

I have phpcs already installed on my text editor so I thought it would be nice to have in this project.

codecov · 2017-06-23T17:06:16Z

Codecov Report

Merging #328 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   98.04%   98.05%   +<.01%     
==========================================
  Files          35       35              
  Lines        2708     2718      +10     
==========================================
+ Hits         2655     2665      +10     
  Misses         53       53

Impacted Files	Coverage Δ
src/Parse/ParseInstallation.php	`100% <ø> (ø)`	⬆️
src/Parse/ParseConfig.php	`100% <ø> (ø)`	⬆️
src/Parse/ParsePushStatus.php	`100% <ø> (ø)`	⬆️
src/Parse/HttpClients/ParseCurl.php	`100% <100%> (ø)`	⬆️
src/Parse/ParseFile.php	`100% <100%> (ø)`	⬆️
src/Parse/Internal/RemoveOperation.php	`100% <100%> (ø)`	⬆️
src/Parse/HttpClients/ParseStream.php	`100% <100%> (ø)`	⬆️
src/Parse/ParseClient.php	`98.93% <100%> (ø)`	⬆️
src/Parse/HttpClients/ParseStreamHttpClient.php	`100% <100%> (ø)`	⬆️
src/Parse/ParseRole.php	`100% <100%> (ø)`	⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20dd7a5...3b23db4. Read the comment docs.

dplewis · 2017-06-23T18:12:34Z

@montymxb for future reference if you want to ignore lines for linter

// @codingStandardsIgnoreStart
echo 'ignore';
// @codingStandardsIgnoreEnd

// @codingStandardsIgnoreLine
echo 'ignore line' // @codingStandardsIgnoreLine

If you want to ignore lines to improve coverage

// @codeCoverageIgnoreStart
echo 'coverage';
// @codeCoverageIgnoreEnd

exit; // @codeCoverageIgnore

montymxb

Whew....that's a lot of linted code...Ok, so everything checks out except most of those ignores that should be corrected. If you can take care of that I'll check out those changes and we should be good!

montymxb · 2017-06-23T17:47:34Z

CONTRIBUTING.md

@@ -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`


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.

montymxb · 2017-06-23T17:49:28Z

CONTRIBUTING.md

+
+Make sure your code is linted with phpcs (PSR-2 Coding Style)
+
+    `npm run lint`


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.

montymxb · 2017-06-23T17:57:18Z

CONTRIBUTING.md

+
+You can automatically fix lint errors with phpcbf
+
+    `npm run lint:fix`


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.

Yeah auto linting is my friend on this one

montymxb · 2017-06-23T18:03:17Z

src/Parse/ParseClient.php

@@ -124,6 +124,7 @@
     *
     * @throws Exception
     */
+    //@codingStandardsIgnoreLine


Just curious what was the error phpcs was bringing up here?

Nvm I took a look at it myself. Considering that this line is rather long I would suggest breaking it down like ParseClient::_request. Then just make sure to remove the ignore above it.

montymxb · 2017-06-23T18:14:59Z

src/Parse/ParseObject.php

@@ -807,6 +806,7 @@ public static function destroyAll(array $objects, $useMasterKey = false)
     */
    private static function destroyBatch(array $objects, $useMasterKey = false)
    {
+        //@codingStandardsIgnoreStart


Same thing here, you can break down the offending string being set on path. Looking it you can probably break it down fairly easily like so. Just be sure to remove the ignore start/end once done.

$data[] = [ 'method' => 'DELETE', 'path' => '/'.ParseClient::getMountPath(). 'classes/'.$object->getClassName(). '/'.$object->getObjectId(), ];

montymxb · 2017-06-23T18:18:16Z

src/Parse/ParseUser.php

-        $auth_token,
-        $auth_token_secret)
-    {
+        $auth_token, //@codingStandardsIgnoreLine


This is fine for an ignore, but if you can put it between like so

$consumer_secret = null, //@codingStandardsIgnoreLine $auth_token,

it should still work and won't be trailing after $auth_token instead.

montymxb · 2017-06-23T18:18:58Z

src/Parse/ParseUser.php

@@ -246,10 +247,11 @@ public static function loginWithAnonymous()
         * @link https://en.wikipedia.org/wiki/Universally_unique_identifier
         */
        $uuid_parts = str_split(md5(mt_rand()), 4);
+        //@codingStandardsIgnoreStart


Same, please break the offending line down and remove the ignores.

montymxb · 2017-06-23T18:20:12Z

src/Parse/ParseUser.php

@@ -338,10 +342,11 @@ public function linkWithTwitter(
        $screen_name,
        $consumer_key,
        $consumer_secret = null,
-        $auth_token,
+        $auth_token, //@codingStandardsIgnoreLine


Same as the other twitter one, just bump it between the two rather than right after one of the args.

montymxb · 2017-06-23T18:30:27Z

I don't think we'll ever be needing to ignore lines to bump up our coverage, besides I wouldn't really have any desire in artificially inflating that number.

According to codecov's report we're already the top dog at just under 98% total coverage as is 👍 .

flovilmart · 2017-06-23T18:32:26Z

Yeah, don't ignore the not-covered lines it makes it impossible to improve correctly the coverage or identifying potential issues.

dplewis · 2017-06-23T22:07:15Z

@montymxb thanks for the feedback. The changes have been made

montymxb

Alright just a couple nit picky things. Everything else looks good to me though. And with the tests looking okay I think we're ready to merge this in once these final changes have been made.

montymxb · 2017-06-24T01:00:50Z

CONTRIBUTING.md

+
+You may also run tests directly using phpunit as follows:
+
+    npm test

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


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)

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:

montymxb · 2017-06-24T01:03:56Z

CONTRIBUTING.md


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

-    `npm run lint`
+    npm run lint

 You can automatically fix lint errors with phpcbf


And if we can add a colon here at the end of this statement too that would be fantastic.

montymxb · 2017-06-24T01:26:39Z

Cool, thanks! Just waiting on one of the CI jobs to rerun real quick.

* Add lint * update travis * coding style * add coverage to gitignore * removed ignore lines * nit

* Add ParsePolygon Type and polygonContains query * test class fix * error handling for polygon * Update PHP_CodeSniffer (#328) * Add lint * update travis * coding style * add coverage to gitignore * removed ignore lines * nit * Updated parse-php-sdk to version 1.2.9 * Corrects and updates phpdoc references/errors (#329) * Corrects and updates phpdoc references/errors * Lint fixes * Added 'Getting Started' to README.md Adds a **Getting Started** section to README.md to direct newcomers to the [official guide](http://docs.parseplatform.org/php/guide/) and [API reference](http://parseplatform.org/parse-php-sdk/namespaces/Parse.html). * Remove Default API (#332) * Removed default api and added appropriate checking * lint * Pages autodeploy and phpdoc style enforcing (#335) * Added 'document-check' to add phpdoc checking during tests and added deploy for api ref on gh-pages * Wrapping filename in quotes * Moved bash out of package.json * Unescaping strings * Testing missing block comment * Fixing lint exception to expose phpdoc style issue * Restored class summary * removed comment * fix documentation * Pinned jms/serializer to 1.7.1 (#336) * Pinned jms/serializer to 1.7.1 * Checking to update jms/serializer to 1.8.0 ONLY on php 5.4 for travis-ci * Added comment, and added graphviz for class diagrams in generated api docs

dplewis added 4 commits June 23, 2017 02:41

Add lint

e8117ff

update travis

fba5ae0

coding style

35a30ee

add coverage to gitignore

84ab1d7

montymxb suggested changes Jun 23, 2017

View reviewed changes

removed ignore lines

5efc5a2

montymxb suggested changes Jun 24, 2017

View reviewed changes

nit

3b23db4

montymxb approved these changes Jun 24, 2017

View reviewed changes

montymxb merged commit ee8d556 into parse-community:master Jun 24, 2017

dplewis deleted the lint branch June 24, 2017 03:25

dplewis added a commit to dplewis/parse-php-sdk that referenced this pull request Jul 12, 2017

Update PHP_CodeSniffer (parse-community#328)

28b5e52

* Add lint * update travis * coding style * add coverage to gitignore * removed ignore lines * nit

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update PHP_CodeSniffer #328

Update PHP_CodeSniffer #328

dplewis commented Jun 23, 2017 •

edited

Loading

codecov bot commented Jun 23, 2017 •

edited

Loading

dplewis commented Jun 23, 2017

montymxb left a comment

montymxb Jun 23, 2017

montymxb Jun 23, 2017

dplewis Jun 23, 2017

montymxb Jun 23, 2017

dplewis Jun 23, 2017

montymxb Jun 23, 2017

montymxb Jun 23, 2017

montymxb Jun 23, 2017

montymxb Jun 23, 2017

montymxb Jun 23, 2017

montymxb Jun 23, 2017

montymxb commented Jun 23, 2017

flovilmart commented Jun 23, 2017

dplewis commented Jun 23, 2017

montymxb left a comment

montymxb Jun 24, 2017

montymxb Jun 24, 2017

montymxb Jun 24, 2017

montymxb commented Jun 24, 2017


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

		`npm run lint`


		You can automatically fix lint errors with phpcbf

		`npm run lint:fix`

Update PHP_CodeSniffer #328

Update PHP_CodeSniffer #328

Conversation

dplewis commented Jun 23, 2017 • edited Loading

codecov bot commented Jun 23, 2017 • edited Loading

Codecov Report

dplewis commented Jun 23, 2017

montymxb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

montymxb commented Jun 23, 2017

flovilmart commented Jun 23, 2017

dplewis commented Jun 23, 2017

montymxb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

montymxb commented Jun 24, 2017

dplewis commented Jun 23, 2017 •

edited

Loading

codecov bot commented Jun 23, 2017 •

edited

Loading