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

Apply fixes for phpcs, remove unused code. #14

Merged
merged 1 commit into from
Aug 5, 2017

Conversation

travis-bradbury
Copy link
Contributor

This fixes the following issues found by phpcs.

$ phpcs --standard=Drupal . --ignore=.css

FILE: ...home/tbradbury/src/drupal/orange_starter/orange_starter.info.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 1 | WARNING | Remove "version" from the info file, it will be added
   |         | by drupal.org packaging automatically
----------------------------------------------------------------------


FILE: /home/tbradbury/src/drupal/orange_starter/orange_starter.theme
----------------------------------------------------------------------
FOUND 5 ERRORS AND 8 WARNINGS AFFECTING 11 LINES
----------------------------------------------------------------------
   8 | WARNING | [x] Unused use statement
  10 | WARNING | [x] Unused use statement
  41 | ERROR   | [x] TRUE, FALSE and NULL must be uppercase; expected
     |         |     "FALSE" but found "false"
  85 | ERROR   | [x] Short array syntax must be used to define arrays
  87 | WARNING | [x] A comma should follow the last multiline array
     |         |     item. Found: )
  92 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
     |         |     Implements hook_foo_BAR_ID_bar() for
     |         |     xyz_bar().",, "* Implements
     |         |     hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
     |         |     "* Implements hook_foo_BAR_ID_bar() for
     |         |     xyz-bar.tpl.php.", or "* Implements
     |         |     hook_foo_BAR_ID_bar() for block templates."
 102 | ERROR   | [x] Namespaced classes/interfaces/traits should be
     |         |     referenced with use statements
 189 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
     |         |     Implements hook_foo_BAR_ID_bar() for
     |         |     xyz_bar().",, "* Implements
     |         |     hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
     |         |     "* Implements hook_foo_BAR_ID_bar() for
     |         |     xyz-bar.tpl.php.", or "* Implements
     |         |     hook_foo_BAR_ID_bar() for block templates."
 216 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
     |         |     Implements hook_foo_BAR_ID_bar() for
     |         |     xyz_bar().",, "* Implements
     |         |     hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
     |         |     "* Implements hook_foo_BAR_ID_bar() for
     |         |     xyz-bar.tpl.php.", or "* Implements
     |         |     hook_foo_BAR_ID_bar() for block templates."
 228 | WARNING | [ ] Line exceeds 80 characters; contains 98
     |         |     characters
 228 | WARNING | [ ] Format should be "* Implements hook_foo().", "*
     |         |     Implements hook_foo_BAR_ID_bar() for
     |         |     xyz_bar().",, "* Implements
     |         |     hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
     |         |     "* Implements hook_foo_BAR_ID_bar() for
     |         |     xyz-bar.tpl.php.", or "* Implements
     |         |     hook_foo_BAR_ID_bar() for block templates."
 230 | ERROR   | [x] Namespaced classes/interfaces/traits should be
     |         |     referenced with use statements
 230 | ERROR   | [x] Expected 1 space before opening brace; found 0
----------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 100ms; Memory: 8Mb

I also made a couple other changes that removed unused code or, in one case, fixed an issue where a variable may be undefined.

There's a few other things that looked funny:

/**
 * Implements hook_preprocess_taxonomy_term().
 */
function orange_starter_preprocess_taxonomy_term(&$variables) {
  if (isset($variables['term'])) {
    // Set base path variable.
    $variables['base_path'] = base_path();
  }
}

I'm not sure why we'd set base_path in a taxonomy term preprocess but I didn't want to remove it if it's used. If it does belong here then maybe we could change the comment from // Set base path variable. to a quick explanation of what it's for. Right now the comment is redundant.

          // Image title.
          if (!empty($image->title)) {

Some other comments are not very informative or entirely redundant but I didn't make any changes in this PR.

@cbildstein
Copy link
Collaborator

base_path isn't available in all Drupal templates, so that's why it's set for Taxonomy. Drupal includes it for page twig files, but not Taxonomy etc.

Copy link
Contributor

@joshmiller83 joshmiller83 left a comment

Choose a reason for hiding this comment

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

Looks great, I think we should merge!

@cbildstein cbildstein merged commit 392c183 into AcroMedia:8.x-1.x Aug 5, 2017
@travis-bradbury travis-bradbury deleted the code-standards branch August 8, 2017 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants