(NFC) Update various files to pass future civicrm/coder ruleset #13979
Conversation
This file contains 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
Method: * Checkout latest merged branch of civicrm/coder (`8.x-2.x-civi`) * Run this command to autoclean a batch of 100 files `PG=1 SIZE=100 ; find Civi/ CRM/ api/ bin/ extern/ tests/ -name '*.php' | grep -v /examples/ | grep -v /DAO/ | sort | head -n $(( $PG * $SIZE )) | tail -n $SIZE | xargs phpcbf-civi` * Go through the diff. For anything that looks wonky, open in an editor and find a better solution Note: The automated checker makes good points about awkward indentation, but the automated cleanup often makes it worse. So that's why I have to open it up.
Method: * Checkout latest merged branch of civicrm/coder (`8.x-2.x-civi`) * Run this command to autoclean a batch of 100 files `PG=2 SIZE=100 ; find Civi/ CRM/ api/ bin/ extern/ tests/ -name '*.php' | grep -v /examples/ | grep -v /DAO/ | sort | head -n $(( $PG * $SIZE )) | tail -n $SIZE | xargs phpcbf-civi` * Go through the diff. For anything that looks wonky, open in an editor and find a better solution. Note: The automated checker makes good points about awkward indentation, but the automated cleanup often makes it worse. So that's why I have to open it up.
|
(Standard links)
|
seamuslee001
reviewed
Apr 5, 2019
| @@ -30,7 +30,6 @@ | |||
| * @copyright CiviCRM LLC (c) 2004-2019 | |||
| */ | |||
|
|
|||
| use Civi\ActionSchedule\RecipientBuilder; | |||
There was a problem hiding this comment.
@totten this is an interesting one
There was a problem hiding this comment.
Agree. I opened that file to double-check references, and it seemed OK without the use statement. Also, CRM_Activity_ActionMappingTest, has coverage of this file. (There was an @see, but that worked with or without the use.)
|
This looks good to me merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Overview
There are pending updates to the
civicrm/coderruleset which would add a large batch of style issues. This pays down a little of that debt in advance.NOTE: This builds on #13978, which should be probably merged first.
Method
8.x-2.x-civi; aka503b3f4d91b4f37bf6004c5e15e3beb37fed6c6b)PG=2 SIZE=100 ; find Civi/ CRM/ api/ bin/ extern/ tests/ -name '*.php' | grep -v /examples/ | grep -v /DAO/ | sort | head -n $(( $PG * $SIZE )) | tail -n $SIZE | xargs phpcbf-civiNote: The automated checker makes good points about awkward indentation, but the automated cleanup often makes it worse. So that's why I have to open it up.
Note: So far, I've observed that ~33% of scanned files actually raise issues. Processing a batch of 100 files per above takes me about 20-30min.