The Theme Switcher module allows you to create theme-switching rules which allow automatic selection of a theme based on Drupal 8 Conditions system.

Project link

https://www.drupal.org/project/theme_switcher

Git instructions

git clone --branch 8.x-1.x https://git.drupalcode.org/project/theme_switcher.git

PAReview checklist

https://pareview.sh/pareview/https-git.drupal.org-project-theme_switcher.git

Comments

amarincolas created an issue. See original summary.

amarincolas’s picture

Status: Active » Needs review
manish34jain’s picture

Hello Amarincolas,

Please run the automated review and check the errors & warnings.

Git errors:

Review of the master branch (commit a934c50):

  • Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
    • The INTRODUCTION section is missing.
    • The REQUIREMENTS section is missing.
    • The INSTALLATION section is missing.
    • The CONFIGURATION section is missing.
  • Remove the LICENSE, drupal.org packaging will add a LICENSE.txt file automatically.
  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    
    FILE: ...101/web/vendor/drupal/pareviewsh/pareview_temp/theme_switcher.module
    --------------------------------------------------------------------------
    FOUND 4 ERRORS AFFECTING 4 LINES
    --------------------------------------------------------------------------
     10 | ERROR | [x] When importing a class with "use", do not include a
        |       |     leading \
     21 | ERROR | [x] Case breaking statements must be followed by a single
        |       |     blank line
     41 | ERROR | [ ] Inline doc block comments are not allowed; use "/*
        |       |     Comment */" or "// Comment" instead
     58 | ERROR | [ ] Inline doc block comments are not allowed; use "/*
        |       |     Comment */" or "// Comment" instead
    --------------------------------------------------------------------------
    PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
    --------------------------------------------------------------------------
    
    Time: 2.36 secs; Memory: 6Mb
    
  • DrupalPractice has found some issues with your code, but could be false positives.
    
    FILE: ...dor/drupal/pareviewsh/pareview_temp/src/Entity/ThemeSwitcherRule.php
    --------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    --------------------------------------------------------------------------
     13 | WARNING | @author tags are not usually used in Drupal, because over
        |         | time multiple contributors will touch the code anyway
    --------------------------------------------------------------------------
    
    
    FILE: ...r/drupal/pareviewsh/pareview_temp/src/ThemeSwitcherRuleInterface.php
    --------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    --------------------------------------------------------------------------
     10 | WARNING | @author tags are not usually used in Drupal, because over
        |         | time multiple contributors will touch the code anyway
    --------------------------------------------------------------------------
    
    
    FILE: ...rupal/pareviewsh/pareview_temp/src/Theme/ThemeSwitcherNegotiator.php
    --------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    --------------------------------------------------------------------------
     85 | WARNING | Unused variable $machine_name.
    --------------------------------------------------------------------------
    
    
    FILE: ...al/pareviewsh/pareview_temp/src/Form/ThemeSwitcherRuleDeleteForm.php
    --------------------------------------------------------------------------
    FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
    --------------------------------------------------------------------------
     15 | WARNING | @author tags are not usually used in Drupal, because over
        |         | time multiple contributors will touch the code anyway
    --------------------------------------------------------------------------
    
    Time: 1.19 secs; Memory: 4Mb
    
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script.

manish34jain’s picture

Issue summary: View changes
avpaderno’s picture

Assigned: amarincolas » Unassigned
Status: Needs review » Needs work
amarincolas’s picture

I have fixed everything: https://pareview.sh/pareview/http-git.drupal.org-project-theme_switcher-...

Tests will be added on next release.

amarincolas’s picture

Status: Needs work » Needs review
manish34jain’s picture

Hello Amarincolas,

When i install and add theme switcher rule after that it's showing below errors on log.

Warning: call_user_func() expects parameter 1 to be a valid callback, class 'Drupal\theme_switcher\Form\ThemeSwitcherRuleForm' does not have a method 'exist' in Drupal\Core\Render\Element\MachineName::validateMachineName() (line 267 of core\lib\Drupal\Core\Render\Element\MachineName.php)

Thanks

amarincolas’s picture

Hello @manish34jain,

Thank you for your time.

The warning has been fixed.

KuldeepM’s picture

Status: Needs review » Needs work

Hello Amarincolas,

I have tested this module. It is not working with language & webforms conditions. It will set default active theme.

avpaderno’s picture

Status: Needs work » Needs review

Thank you for your review. We don't aim to debug the code, but check what users understand in correctly using the Drupal core APIs, writing secure code, and writing code that follows the Drupal coding standards.

vuil’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the contribution!
I have not found any security related issues into the code.

Just one small suggestion (which does not come from project security) is always better to use try and catch blocks, and handle possible throws and exceptions.
Good job!

amarincolas’s picture

Thank you for your reviews.

@KuldeepM, I have test it with several languages and it works perfectly. Can you give some steps to reproduce? However I have not yet test it with the webform module but I will (It may not make sense with certain conditions like webform).

Thanks.

avpaderno’s picture

Assigned: Unassigned » avpaderno

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.

avpaderno’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.