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.

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

Touch up documentation for phpcs locally #1897

Closed
wants to merge 1 commit into from
Closed

Touch up documentation for phpcs locally #1897

wants to merge 1 commit into from

Conversation

rosiel
Copy link
Member

@rosiel rosiel commented Sep 10, 2021

Purpose / why: #1620

There was worry that phpcs was out of sync between travis and local setups. I verified (to the best of my knowledge without running an actual PR) that there is documentation for running phpcs locally, and that (and how!) it matches what's running in Travis.

What changes were made?

  • Put the specific excludes/filetype options in the main code examples
  • Simplified the examples (removed repeated references to the drupal root folder vs the drupal web folder)
  • Was explicit about how this is installed like this in Vagrant and installing on other systems may vary.

Verification

Does it match? Also, is there more (about doing this in ISLE) that needs to be included?

Interested Parties

Replace this text - Name some folks who may be interested, if documentation related mention @Islandora/documentation, or, if unsure, @Islandora/8-x-committers


Checklist

Pull-request reviewer should ensure the following

  • Does this PR link to related issues?
  • Does the proposed documentation align with the Islandora Documentation Style Guide?
  • Are the changes accurate, useful, free of typos, etc?
  • Does this PR update the last updated on date on the documentation page?

Person merging should ensure the following

  • Does mkdocs still build successfully? (This is indicated by TravisCI passing. To test locally, and see warnings, see How To Build Documentation.)
  • If pages are renamed or removed, have all internal links to those pages been fixed?
  • If pages are added, have they been linked to or placed in the menu?
  • Did the PR receive at least one approval from a committer, and all issues raised have been addressed?

If using a non-Ansible method of deployment, you may need to install phpcs and coder yourself, and the paths may vary.
Copy link
Member

Choose a reason for hiding this comment

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

This is a helpful bit of documentation if you're not using the ansible stuff

* from within Drupal's root directory: `./vendor/bin/phpcs --standard=./vendor/drupal/coder/coder_sniffer/Drupal modules/contrib/my_module`, where `modules/contrib/my_module` is the relative or full path to the PHP file you want to check.
* from within Drupal's `web` directory: `../vendor/bin/phpcs --standard=../vendor/drupal/coder/coder_sniffer/Drupal yourfile`, where `yourfile` is the relative or full path to the PHP file you want to check.
* from within Drupal's root directory: `./vendor/bin/phpcs --standard=./vendor/drupal/coder/coder_sniffer/Drupal --ignore=*.md --extensions=php,module,inc,install,test,profile,theme,css,info web/modules/contrib/my_module`
* from within Drupal's `web` directory: `../vendor/bin/phpcs --standard=../vendor/drupal/coder/coder_sniffer/Drupal --ignore=*.md --extensions=php,module,inc,install,test,profile,theme,css,info modules/contrib/my_module`
Copy link
Member

Choose a reason for hiding this comment

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

we also use the DrupalPractice library

Copy link
Member Author

Choose a reason for hiding this comment

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

@elizoller I thought i knew what you meant but I'm not sure that I do!

In https://github.com/Islandora/islandora_ci/blob/main/travis_scripts.sh we run two commands: phpcs --standard=Drupal and phpcpd. The second one looks to me to be a "copy paste detector".

Looking into Drupal's docs on PHP CodeSniffer CommandLine Usage, they mention "Check Drupal Best Practices" with the example phpcs --standard=DrupalPractice.

I don't see where we actually run this in our code checks. From my knowledge, what's in travis_scripts.sh in the islandora_ci library is the code check that applies to all our repos.

@dannylamb or @whikloj might also know?

@manez
Copy link
Member

manez commented Mar 9, 2022

@rosiel we reviewed this in DIG and wanted to ask if you still wanted to keep working on this one, given that it's pending requested changes and @elizoller is no longer active?

@rosiel rosiel closed this Mar 10, 2022
@rosiel rosiel deleted the rosiel-phpcs branch March 10, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants