? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
28 Jun 2022

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 install if 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.

avatar richard67 richard67 - open - 28 Jun 2022
avatar richard67 richard67 - change - 28 Jun 2022
Status New Pending
avatar richard67 richard67 - change - 28 Jun 2022
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 28 Jun 2022
Category Unit Tests
avatar richard67 richard67 - change - 28 Jun 2022
Labels Added: ?
avatar richard67 richard67 - change - 28 Jun 2022
The description was changed
avatar richard67 richard67 - edited - 28 Jun 2022
avatar richard67
richard67 - comment - 28 Jun 2022

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.

avatar HLeithner
HLeithner - comment - 28 Jun 2022

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

avatar richard67
richard67 - comment - 28 Jun 2022

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?

avatar HLeithner
HLeithner - comment - 28 Jun 2022

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?

avatar richard67
richard67 - comment - 28 Jun 2022

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.

avatar HLeithner
HLeithner - comment - 28 Jun 2022

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...

avatar richard67
richard67 - comment - 28 Jun 2022

@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.

avatar richard67
richard67 - comment - 28 Jun 2022

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 Here it works when I let phpcs check that file only.

avatar joomla-cms-bot joomla-cms-bot - change - 28 Jun 2022
Category Unit Tests
avatar richard67
richard67 - comment - 28 Jun 2022

I give it up. The tests work on my local environment and fail in drone.

avatar richard67 richard67 - close - 28 Jun 2022
avatar richard67 richard67 - change - 28 Jun 2022
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2022-06-28 13:40:29
Closed_By richard67
Labels Removed: ?
avatar HLeithner
HLeithner - comment - 28 Jun 2022

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)

Add a Comment

Login with GitHub to post a comment