phpcs-patch #1
phpcs-patch #1
Conversation
| * @package PHP_CodeSniffer | ||
| * @author Jack Bates <ms419@freezone.co.uk> | ||
| * @author Greg Sherwood <gsherwood@squiz.net> | ||
| * @author Weston Ruter <weston@x-team.com> |
There was a problem hiding this comment.
@shadyvb actually this should be you 😄
There was a problem hiding this comment.
@westonruter Modified in f40fe46, Thanks!
| // If supplied a transaction, or running on server, use svnlook, to handle svn:pre-commit ( on svn server ) | ||
| if ( in_array( '-t', $values['vcsArgs'] ) || ! $values['direct-call'] ) { | ||
| $command = sprintf( | ||
| 'svnlook changed %s', |
There was a problem hiding this comment.
@shadyvb ah, here we're not using PHP_CODESNIFFER_SVNLOOK anyway.
There was a problem hiding this comment.
@westonruter that's a glitch, will switch to using the const till we get a response from squizlabs.
| ? $rev = str_replace( ':', ' ', $vcs_rev ) | ||
| : ''; | ||
| $command = sprintf( | ||
| 'git show %s:%s', |
There was a problem hiding this comment.
@shadyvb if I invoke phpcs-patch with a commit-span like:
phpcs-patch --standard=WordPress -r e82920f:31174f73Then I see among the command log:
== Executing: git show e82920f 31174f73:path/to/file.php
This seems to produce the incorrect results, including a patch followed by the file at the commit 31174f73, I believe.
So isn't the need to ensure that only the recent-most commit is used for git-show?
There was a problem hiding this comment.
@shadyvb for example:
diff --git a/scripts/phpcs-patch b/scripts/phpcs-patch
index 939a7ae..73e06bc 100755
--- a/scripts/phpcs-patch
+++ b/scripts/phpcs-patch
@@ -310,7 +310,7 @@ class PHP_CodeSniffer_Patch_CLI extends PHP_CodeSniffer_CLI
if ( $values['vcsType'] == 'git' ) {
// Get file from a certain revision, fallback to STAGED if not provided
$rev = ( $vcs_rev )
- ? $rev = str_replace( ':', ' ', $vcs_rev )
+ ? $rev = preg_replace( '/.*:/', '', $vcs_rev )
: '';
$command = sprintf(
'git show %s:%s',
…mit hook. Also reworked the way the phpcs script works to make it easier to wrap it with other functionality. git-svn-id: http://svn.php.net/repository/pear/packages/PHP_CodeSniffer/trunk@254739 c90b9560-bf6c-de11-be94-00142212c4b1
| $rev = ( intval( $info ) - 1 ) . ':' . 'HEAD'; | ||
| } | ||
| $command = sprintf( | ||
| 'svn diff -r %s %s --summarize', |
There was a problem hiding this comment.
@shadyvb what if the user just wants to apply the sniffs against the changes which are not committed at all? For Git, this is accomplished with --staged. But since SVN lacks a stage, we need to just look at the currently modified files. So if no revision is provided, then instead of looking for the most recent commit (as done in the else above), shouldn't it omit the -r parameter entirely?
There was a problem hiding this comment.
@westonruter As far as i remember, I believe the assumption of the user testing staged changes is heavily considered throughout the code. We'll need some changes to make this work for non-staged changes as well, probably starting with a new parameter to distinguish which changes to check, and some code changes/testing.
|
I talked with @aaronjorbin about getting the PHP_CodeSniffer WordPress-Coding-Standards added to WordPress core develop repo, and invoked automatically by grunt for pre-commit checks. This All that to say, I want to some more testing on your great work here @shadyvb and then open a PR for merging upstream into the PHP_CodeSniffer project. Once that is done, then we can open a WP Trac ticket for getting this integrated into the dev tools! |
|
@westonruter This seems like a great thing to do! Let me know if i can do anything to help! |
|
Opened a Trac ticket for this: https://core.trac.wordpress.org/ticket/30153 |
ie: phpcs-patch --standard=WordPress . --report=changeset
|
Closing in favor of #5 |
|
Awesome stuff. Are you planning on submitting this upstream to PHP_CodeSniffer? |
|
@aaronjorbin yes, #5 is the changes we'd upstream. |
Pull master from squizlabs
No description provided.