Instruct wp-dev-lib to check all files not just those in patch#1822
Merged
westonruter merged 1 commit intodevelopfrom Jan 17, 2019
Merged
Instruct wp-dev-lib to check all files not just those in patch#1822westonruter merged 1 commit intodevelopfrom
westonruter merged 1 commit intodevelopfrom
Conversation
Member
Author
|
See from Travis build that PHPCS is now being invoked even without any PHP files being touched: https://travis-ci.org/ampproject/amp-wp/jobs/480632846#L381-L382 The downside to this change is that the build will take longer, but it seems justified for the sake of consistency. |
1 task
felixarntz
approved these changes
Jan 17, 2019
Collaborator
|
LGTM. However, for an eventual pre-commit hook we should ensure PHPCS still only runs on the changed files to speed that up. For CI I agree it's not so important as consistency. |
Member
Author
|
Agreed. For that perhaps we should do something like this: if [ -z $TRAVIS ]; then
CHECK_SCOPE=all
else
CHECK_SCOPE=changed-files
fi |
Member
Author
westonruter
added a commit
that referenced
this pull request
Feb 1, 2019
…velop-to-1.0.2 * 'develop' of github.com:ampproject/amp-wp: (26 commits) Fix PEAR.Functions.FunctionCallSignature phpcs issues Strip old-school CDATA and HTML comments from XHTML-compatible style elements Re-simplify condition include_manifest_comment; remove undefined constant Add tests for style sanitizer include_manifest_comment option Introduce when_css_excluded option for include_manifest_comment arg Suppress style[amp-custom] manifest HTML comment when not WP_DEBUG Add missing vendor deps to build after #1809 Update dependency xwp/wp-dev-lib to v1.0.1 Add wp_generator to classic post template Remove Powered by WordPress in classic footer template Fix broken unit test as it is just as valid now. Remove unnecessary strlen() call. Ignore the home URL's path if present when normalizing image URLs. Go back to using site_url() in AMP_Style_Sanitizer and harden concatenation of relative URLs. Remove redundant composer install Only set CHECK_SCOPE=all on Travis (see #1822) Improve sanity check instructions in release steps Pin wp-dev-lib to 1.0.0 Eliminate references to submodules from contributing.md Remove obsolete DEV_LIB_SKIP of composer ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After #1810 we no longer need to limit PHPCS to report issues on the lines that were changes in a given PR, which is the default behavior of wp-dev-lib. This was needed because the repo did not adhere to the standards, and so the patch scope limitation allowed us to make incremental improvements over time. This is no longer needed, and the patch scope limitation is going to cause us problems going forward such as when versions of WPCS are updated by Renovate (e.g. #1814), since the
composer.jsonchange wouldn't triggerphpcsby default.