Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Automate squizlabs/php_codesniffer #32785

Merged
merged 2 commits into from
Sep 28, 2018
Merged

Automate squizlabs/php_codesniffer #32785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba23fec
Automate squizlabs/php_codesniffer
phil-davis Sep 20, 2018
528ad27
phpcs style changes for master
phil-davis Sep 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ help:
@echo -e "make clean\t\t\tclean everything"
@echo -e "make install-composer-deps\tinstall composer dependencies"
@echo -e "make update-composer\t\tupdate composer.lock"
@echo -e "make install-nodejs-deps\t\tinstall Node JS and Javascript dependencies"
@echo -e "make install-nodejs-deps\tinstall Node JS and Javascript dependencies"
@echo
@echo -e "Note that running 'make' without arguments already installs all required dependencies"
@echo
Expand Down Expand Up @@ -196,6 +196,7 @@ test-php-lint: $(composer_dev_deps)
.PHONY: test-php-style
test-php-style: $(composer_dev_deps)
$(composer_deps)/bin/php-cs-fixer fix -v --diff --diff-format udiff --dry-run --allow-risky yes
$(composer_deps)/bin/phpcs --runtime-set ignore_warnings_on_exit --standard=phpcs.xml tests/acceptance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, so we won't be using the same rules for QA code ?

can you make a separate ticket to consolidate said rules so we only use a single tool ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand your question.
php-cs-fixer implements some "basic" rules across the whole code-base, including acceptance test code. That is already in place.

phpcs has even more code style rules than php-cs-fixer.

During discussions about code style, PHPdoc block etc standards some time ago, the QA team implemented some phpcs rules to get started, and applied it to the tests/acceptance/... folders. This is an "interim" standard while we await the "whole project" deciding on a project-wide standard. We have been manually running phpcs locally on PRs to see that we meet our standard, but of course we often forget!

So automating it here will force us (and anyone who touches tests/acceptance/.../*.php) to remember.

If the whole project some day decides to adopt a standard that is inclusive of these rules, then easy - the acceptance test code already meets the rules here. If the project adopts some standard that is not the same/similar, then we reformat doc blocks or whatever (like the rest of the project will have to do anyway).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm ok, so if phpcs itself has more rules than phpcsfixer we could eclipse phpcsfixer then... and many IDEs support loading phpcs rules but not ones from phpcsfixer...

php build/OCPSinceChecker.php

.PHONY: test-php-style-fix
Expand Down
1 change: 1 addition & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"jarnaiz/behat-junit-formatter": "^1.3",
"mikey179/vfsStream": "^1.6",
"owncloud/coding-standard": "^1.0",
"squizlabs/php_codesniffer": "3.*",
"phpunit/phpunit": "^5.7",
"rdx/behat-variables": "^1.2",
"roave/security-advisories": "dev-master",
Expand Down
68 changes: 65 additions & 3 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions phpcs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
<exclude name="Generic.WhiteSpace.DisallowTabIndent.NonIndentTabsUsed" />
</rule>

<rule ref="Generic.Files.LineLength">
<exclude name="Generic.Files.LineLength.TooLong" />
</rule>

<rule ref="PEAR">
<exclude name="Generic.Commenting.DocComment.ShortNotCapital" />
<exclude name="Generic.Commenting.DocComment.SpacingAfter" />
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/features/bootstrap/BasicStructure.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1657,7 +1657,7 @@ public function setupLocalStorageBefore() {
'local',
'null::null',
'-c',
'datadir=' . $this->getServerRoot(). '/' . LOCAL_STORAGE_DIR_ON_REMOTE_SERVER
'datadir=' . $this->getServerRoot() . '/' . LOCAL_STORAGE_DIR_ON_REMOTE_SERVER
]
);
// stdOut should have a string like "Storage created with id 65"
Expand Down
4 changes: 2 additions & 2 deletions tests/acceptance/features/lib/FilesPageElement/DetailsDialog.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,13 @@ private function findDetailsTab($tabName) {
*/
public function changeDetailsTab($tabName) {
$tabId = $this->getDetailsTabId($tabName);
$tabSwitchXpath = "//li[@data-tabid='".$tabId."']";
$tabSwitchXpath = "//li[@data-tabid='" . $tabId . "']";
$tabSwitch = $this->find("xpath", $tabSwitchXpath);
if ($tabSwitch === null) {
throw new ElementNotFoundException(
__METHOD__ .
" could not find tab switch with id $tabName"
);
);
}
$this->waitTillElementIsNotNull($tabSwitchXpath);
$tabSwitch->focus();
Expand Down
2 changes: 1 addition & 1 deletion tests/acceptance/features/lib/FilesPageElement/FileActionsMenu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public function openDetails() {
throw new ElementNotFoundException(
__METHOD__ .
" could not find action button with label $this->deleteActionLabel"
);
);
}
$detailsBtn->focus();
$detailsBtn->click();
Expand Down