-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Add build folder to PHPCS checks #38168
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.
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
Add build folder to PHPCS checks #38168
Conversation
This reverts commit 3d5edfb.
…thub.com/richard67/joomla-cms into 4.2-dev-add-build-folder-to-pcpcs-checks
|
The strange thing is that PHPCS fails in drone here for this PR: https://ci.joomla.org/joomla/joomla-cms/56045/1/6 . But when I run the test locally it works. @HLeithner Do you have an idea what the problem could be? My PR doesn't even touch that "build/psr12/phpcs.joomla.report.php" file. |
it's possible that the file pattern matcher works different under windows and linux |
@HLeithner Here I have Linux, and Drone where it fails has Linux, too, or am I wrong? |
hard to say can you fix the file? |
@HLeithner I see no mistake in the file. The "build/psr12/phpcs.joomla.report.php" file is not in any of the exceptions, so it is checked for all. Can it be that this file is used by the checker itself? In this case we would have to exclude it. |
|
no the report is not used in the check, but you can use phpcs directly on this file if it is included for unknown reasons... |
|
@HLeithner Another thing is that if you prefer to have the exception in the single files and have some comment there, I have no idea what kind of comment you except e.g. for the build/build.php file. Maybe I should close this PR and leave that to someone else. Or you make me some suggestion for the right comment. |
@HLeithner Here it works when I let phpcs check that file only. |
This reverts commit d1aff0e.
|
I give it up. The tests work on my local environment and fail in drone. |
|
for example the build/build.php needs the exception because it has 3 functions and a constant declaration. All of them can be removed because they are use less (functions used only once and the PHP_TAB constant doesn't makes it better) |
Pull Request for Issue # .
Summary of Changes
Removes the build folder from the exceptions in ruleset.xml so the PHPCS checks are run on that folder by default, too, and adds the necessary exceptions to the "PSR1.Files.SideEffects.FoundWithSymbols" rule to the right section in the ruleset.xml file.
In addition, the exception to that rule for the "tests/Unit/bootstrap.php" file has been moved from the file to the ruleset.xml.
Testing Instructions
On a clean, current 4.2-dev run
composer installif not done before.Then run
./libraries/vendor/bin/phpcs --extensions=php -p --standard=ruleset.xml .Actual result BEFORE applying this Pull Request
The "./build" folder is not checked. There are no errors or warnings.
When running
./libraries/vendor/bin/phpcs --extensions=php -p --standard=ruleset.xml ./build, the build folder will be checked, and there will be warnings as mentioned in PR #38167 .Expected result AFTER applying this Pull Request
The "./build" folder is checked. There are no errors or warnings.
Documentation Changes Required
None.