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.

Sign up for GitHub

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

Jump to bottom

Update to Coding Standard command #469

Merged
merged 3 commits into from
Jun 2, 2017
Merged

Update to Coding Standard command #469

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9962dae
Update to Coding Standard command
eeree May 24, 2017
5275df2
Update CONTRIBUTING.md
eeree Jun 1, 2017
7ab8832
Update CONTRIBUTING.md
eeree Jun 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Make sure your code changes comply with the PSR-2 coding standard by
using [PHP Codesniffer](https://github.com/squizlabs/PHP_CodeSniffer)
from within your PHPMD folder:

vendor/bin/phpcs -p --extensions=php --standard=PSR2 src > phpcs.txt
vendor/bin/phpcs -p --extensions=php --standard=PSR2 --ignore=src/tests/resources $(git ls-files -om --exclude-standard | grep '\.php$') > phpcs.txt
Copy link
Member

Choose a reason for hiding this comment

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

Three thoughts:

  1. grep is not available on Windows, so I prefer to keep the current one. (yes i know vendor/bin/phpcs needs to be adjusted on Windows, anyway)
  2. Just because they aren't changed, doesn't mean those files don't contain a cs violation. The project isn't THAT huge, anyway.
  3. I would prefer, if we could align the commands with those used as Composer scripts as much as possible:

https://github.com/phpmd/phpmd/blob/master/composer.json#L54

So it would become

vendor/binphpcs -p --standard=PSR2 ./src/main/php ./src/test/php > phpcs.txt

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, we can state, that one could use the Composer scripts composer cs-check and composer cs-fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ravage84 ,
Current problem is, that there are some files, that already violates the styling rules. That was my main goal here. Grep is not a big deal (I forgot about Windows users) - git ls-files gives an --ignore flag, that works exactly as | grep -v (but again - Windows users will have to execute that in two commands). I assume, that I can just decline this PR, mostly because of OS incompatibility.

Copy link
Member

Choose a reason for hiding this comment

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

How about you change your PR to add a note for Linux users who are able to use this, containing the optimized cli command?


Check the ``phpcs.txt`` once it finished.
Copy link
Member

Choose a reason for hiding this comment

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

Add another white line before.


Expand Down