Adds theme namespaces to the "containers.namespaces" and "class_loader"
services. This allows themes to define classes and have discoverable annotated
plugin definitions.

Theme namespaces are added with the pattern Drupal\Theme\<theme name> to
keep theme namespaces differentiated from the standard Drupal module namespaces
Drupal\<module name>.

Module also includes plugin definition and discovery classes to assist with creating
plugins that can be loaded based on the active theme.

Project link

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

Git instructions

git clone --branch '1.0.x' https://git.drupalcode.org/project/themespace.git

Comments

lemming created an issue. See original summary.

lemming’s picture

Issue summary: View changes
avpaderno’s picture

Issue summary: View changes

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smother review.

To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.

andrei.vesterli’s picture

Hello @lemming

I've tested your module and it does a great job!

The only 2 things I see are:

1. Please, adjust the composer.json according to this documentation.

2. I've ran the tests via command: php scripts/run-tests.sh themespace (from the core folder) and got this:

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\themespace\Kernel\PluginDiscoveryTest
  - Drupal\Tests\themespace\Kernel\PluginManagerTest
  - Drupal\Tests\themespace\Kernel\ThemeNamespaceTest
  - Drupal\Tests\themespace\Unit\PluginDefinitionTest

Test run started:
  Monday, May 2, 2022 - 00:10

Test summary
------------

PHP Fatal error:  Uncaught Error: Class "Drupal\Tests\themespace\Kernel\PluginDiscoveryTest" not found in /var/www/html/web/core/scripts/run-tests.sh:835
Stack trace:
#0 /var/www/html/web/core/scripts/run-tests.sh(60): simpletest_script_run_one_test('38', 'Drupal\\Tests\\th...')
#1 {main}
  thrown in /var/www/html/web/core/scripts/run-tests.sh on line 835

Fatal error: Uncaught Error: Class "Drupal\Tests\themespace\Kernel\PluginDiscoveryTest" not found in /var/www/html/web/core/scripts/run-tests.sh:835
Stack trace:
#0 /var/www/html/web/core/scripts/run-tests.sh(60): simpletest_script_run_one_test('38', 'Drupal\\Tests\\th...')
#1 {main}
  thrown in /var/www/html/web/core/scripts/run-tests.sh on line 835
FATAL Drupal\Tests\themespace\Kernel\PluginDiscoveryTest: test runner returned a non-zero error code (255).
Drupal\Tests\themespace\Kernel\PluginDiscoveryTest             0 passes   1 fails                            

FATAL Drupal\Tests\themespace\Kernel\PluginDiscoveryTest: Found no database prefix for test ID 38. (Check whether setUp() is invoked correctly.)Drupal\Tests\themespace\Kernel\PluginManagerTest               2 passes                                      
PHP Fatal error:  Uncaught Error: Class "Drupal\Tests\themespace\Kernel\ThemeNamespaceTest" not found in /var/www/html/web/core/scripts/run-tests.sh:835
Stack trace:
#0 /var/www/html/web/core/scripts/run-tests.sh(60): simpletest_script_run_one_test('40', 'Drupal\\Tests\\th...')
#1 {main}
  thrown in /var/www/html/web/core/scripts/run-tests.sh on line 835

Fatal error: Uncaught Error: Class "Drupal\Tests\themespace\Kernel\ThemeNamespaceTest" not found in /var/www/html/web/core/scripts/run-tests.sh:835
Stack trace:
#0 /var/www/html/web/core/scripts/run-tests.sh(60): simpletest_script_run_one_test('40', 'Drupal\\Tests\\th...')
#1 {main}
  thrown in /var/www/html/web/core/scripts/run-tests.sh on line 835
FATAL Drupal\Tests\themespace\Kernel\ThemeNamespaceTest: test runner returned a non-zero error code (255).
Drupal\Tests\themespace\Kernel\ThemeNamespaceTest              0 passes   1 fails                            

FATAL Drupal\Tests\themespace\Kernel\ThemeNamespaceTest: Found no database prefix for test ID 40. (Check whether setUp() is invoked correctly.)Drupal\Tests\themespace\Unit\PluginDefinitionTest              4 passes                                      

Test run duration: 4 sec

my stack:
core: 9.3.12
PHP: 8.0
PHPUnit: 9.5.20
drupal-dev: 9.3.x
simpletest-simpletest: 3.x-dev@dev
phpspec/prophecy-phpunit: ^2.

anyway, some of the classes seem to be as not found. Probably, I miss something but this is what I have.

Regards,
Andrei

andrei.vesterli’s picture

Status: Needs review » Needs work
lemming’s picture

Status: Needs work » Needs review

@andrei.vesterli thanks for reviewing.

  1. I've update the composer.json file to match the formatting described.
  2. I've also added some new commits which fix the issues with the test you pointed out here. Looks like it was a typo with my namespace. Oddly it worked fine when I was using PHPUnit to run the tests. I also needed to make some adjustments to the test to be compatible with the SimpleTest 3.x-dev, which again I did not encounter when running with PHPUnit command.

I appreciate the thoughtful review and keeping the community growing.

andrei.vesterli’s picture

@lemming

Hmm, it seems to be much better! Thx for the fast feedback!

A would suggest you use the automated testing section for your project page once you have these tests. Also, run them in the dev branch and configure them to run weekly. This is really good to have once you'll get some development stuff done or something new merged.

Regards,
Andrei

lemming’s picture

Issue summary: View changes
lemming’s picture

Updated the repository default branch and the Git instructions to use the "1.0.x" branch.

avpaderno’s picture

Assigned: Unassigned » avpaderno
Status: Needs review » Fixed

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.

Status: Fixed » Closed (fixed)

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