@Mattixonix

Hi @FlutterStack, thank you for reporting that issue. Problem happens on PHP lower than 8.2. My bad because i didn't tested lower PHP versions after adding trait to optimize code.

Thanks @sarwan_verma for your patch, however it's not working properly. By changing constants to static variables and changing their names You solved global PHP problem, but module will not work after that. With changing variables names you should also change each place where they were used in code. Despite that i'll grand you credit for good idea ;)

I've added new release to module here https://www.drupal.org/project/re_mgr/releases/1.0.6 → . It's fix this problem and also few smaller bugs.

Mattixonix → created an issue.

Thanks again for review. I've extended all function and properties descriptions. I'm not a big fan of this way, because everything is typed so it's very clear what types i am using, still this is the rules and they are better then chaos ;)

About hooks, i did not removed longer description because as You said they are not mandatory and i really like when i see from the description what is going on right there not only knowing what hook is implemented. Also i know that i can comment inside function on specific line but rest of lines usually are used for this purpose mention in function description so i stay with this way.

Thank You for updating my account and all reviewers cause i learned a lot from this!

Hi again @amourow!

About fixes, that's my fault i look for released version, not the newest dev one.

About error, how to reproduce:

1. Install fresh Drupal 10.1.6
2. Install and enable Generative Summary module
3. In Simple Page node structure body field add Enable Generative Summary
4. Do not edit any settings - out of the box
5. Create Simple page and try generate summary

In first attempt i had to increase max_filesize because 2M was not enough.
Then sometimes i had error from console that You are trying to add message on null response.
Also sometimes i had error with "The 'edit any page content' permission is required." if You will see network tab and try display response, worth mention that im on admin account.

Do i need any API key or API organization? If yes they should be required.

Hi @AstonVictor, here are some tips from me:

• In all php files (.module and .install files too) it's good practice to type function arguments and return types
• .module - In all functions there should be both - information about implemented hook and short function description what it does, separated by single empty line of comment
• You should check all php files with proper configured phpStan. Eg. in .module:151 $entity->id() does not always return int, sometimes its even null, then it will break your page, so please check your module with phpStan more info about installation and use here https://www.drupal.org/docs/develop/development-tools/phpstan →
• .routing.yml - _admin_route option is not necessary cause everything after /admin is already using administration page
• composer.json - worth to add what php version your module support
• FieldExtraForm.php:72 - for better styling it's nice to write this if() like this
  

        if (
          in_array('Drupal\Core\Entity\FieldableEntityInterface', class_implements($definition->getOriginalClass())) &&
          in_array('Drupal\user\EntityOwnerInterface', class_implements($definition->getOriginalClass()))
        ) {
          $options[$definition->id()] = $definition->getLabel()->render();
        }

PhpCs is clear, everything looks fine for me, i tested module and works as designed, good work! :D

Hi @amourow, catch few tips from me ;):

• .info.yml file - change package to something more specific
• .info.yml file - You are including js file (if im right) to all pages and this is very bad practice, optimize that by creating .libraries.yml file and set library for this file, then include this library only where it is used, i believe in your case in edit entities forms only when that specific field is used
• .module - add @file description, ;
• .module - add functions descriptions with what they implements and also add short description why this hook is invoke but separate implementation information from short desc with single empty line
• .module - type functions arguments - it's very good practice
• .module - i see that You are using hook_form_alter() and then it starts from if() checking for proper $form_id, in this case to have cleaner code use hook_form_FORM_ID_alter() instead
• .module - remove AjaxResponse, InvokeCommand, GenerativeSummaryController form use section
• composer.json - is your module works fine with php7.3 to 8.2? Is it tested? Im asking because You support D9 and D10. D9.0 works even with php7.3 and some people will want to use this module on this php version. It's better to set php requirement in composer file, also You can change support in info.yml file form D9 to D9.5, but first test module on this versions.
• .routing.yml - there is options _admin_route: TRUE, it's not needed because everything after /admin use administrations theme
• In src files it's nice to type arguments and return types even if they are describe in @param | @return
• Check whole module and add class descriptions
• You can't use code like that \Drupal::messenger(), use Dependency Injections instead, everytime You are in OOP code, see more here https://www.drupal.org/docs/drupal-apis/services-and-dependency-injectio... →
• When i tried to use this module to generate summary i get ajax error like this ""The 'edit any page content' permission is required." and that is very strange cause i'm on admin account

Please fix also phpcs errors:

FILE: /opt/drupal/web/modules/contrib/generative_summary/generative_summary.module
----------------------------------------------------------------------------------
FOUND 3 ERRORS AND 3 WARNINGS AFFECTING 6 LINES
----------------------------------------------------------------------------------
   3 | ERROR   | [ ] Missing short description in doc comment
   7 | WARNING | [x] Unused use statement
   8 | WARNING | [x] Unused use statement
  11 | WARNING | [x] Unused use statement
  22 | ERROR   | [x] Missing function doc comment
 108 | ERROR   | [x] Missing function doc comment
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: /opt/drupal/web/modules/contrib/generative_summary/generative_summary.info.yml
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------


FILE: /opt/drupal/web/modules/contrib/generative_summary/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 10 WARNINGS AFFECTING 10 LINES
----------------------------------------------------------------------
  1 | WARNING | Line exceeds 80 characters; contains 429 characters
  4 | WARNING | Line exceeds 80 characters; contains 114 characters
  5 | WARNING | Line exceeds 80 characters; contains 126 characters
  6 | WARNING | Line exceeds 80 characters; contains 165 characters
  7 | WARNING | Line exceeds 80 characters; contains 127 characters
  8 | WARNING | Line exceeds 80 characters; contains 125 characters
  9 | WARNING | Line exceeds 80 characters; contains 143 characters
 12 | WARNING | Line exceeds 80 characters; contains 88 characters
 13 | WARNING | Line exceeds 80 characters; contains 170 characters
 14 | WARNING | Line exceeds 80 characters; contains 86 characters
----------------------------------------------------------------------


FILE: /opt/drupal/web/modules/contrib/generative_summary/src/Form/GenerativeSummaryConfigForm.php
--------------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
--------------------------------------------------------------------------------------------------------------
   8 | ERROR | [x] Missing class doc comment
  12 | ERROR | [x] Do not append variable name "$models" to the type declaration in a member variable comment
  26 | ERROR | [x] Missing function doc comment
  30 | ERROR | [x] Missing function doc comment
  34 | ERROR | [x] Missing function doc comment
 143 | ERROR | [x] Missing function doc comment
--------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------


FILE: /opt/drupal/web/modules/contrib/generative_summary/src/Plugin/Field/FieldWidget/TextareaWithGenerativeSummaryWidget.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------
 74 | ERROR   | [x] Missing function doc comment
 85 | WARNING | [ ] Unused variable $max_chunk_tokens.
-----------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------------


FILE: /opt/drupal/web/modules/contrib/generative_summary/src/Controller/GenerativeSummaryController.php
-------------------------------------------------------------------------------------------------------
FOUND 19 ERRORS AND 4 WARNINGS AFFECTING 20 LINES
-------------------------------------------------------------------------------------------------------
   9 | ERROR   | [x] Missing class doc comment
  11 | ERROR   | [x] Missing function doc comment
  18 | ERROR   | [ ] Missing parameter comment
  19 | ERROR   | [ ] Missing parameter comment
  20 | ERROR   | [x] Separate the @param and @return sections by a blank line.
  20 | ERROR   | [ ] Description for the @return value must be on the next line
  50 | ERROR   | [x] Missing function doc comment
  52 | ERROR   | [x] Expected 1 blank line after function; 2 found
  60 | ERROR   | [x] Separate the @param and @return sections by a blank line.
  78 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  83 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  88 | ERROR   | [ ] Missing short description in doc comment
  89 | ERROR   | [ ] Missing parameter comment
  89 | ERROR   | [ ] Missing parameter type
  90 | ERROR   | [ ] Missing parameter comment
  90 | ERROR   | [ ] Missing parameter type
  91 | ERROR   | [ ] Description for the @return value is missing
  94 | ERROR   | [x] Opening brace should be on the same line as the declaration
 120 | ERROR   | [x] Expected newline after closing brace
 121 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
 126 | ERROR   | [x] Missing function doc comment
 132 | ERROR   | [x] Missing function doc comment
 135 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------------------------------

It is really nice module in my opinion and i'm sorry i couldn't use it proper, hope You will fix this ;)

Good to know @apaderno. Then @Rachel Jaro, You should rename LICENCE to LICENCE.txt because currently i downloaded module with those two files.

Hi @Rachel Jaro, here are few tips from me:

• You should remove LICENCE file, because it is generated automatically
• You can create headers in README base on this doc - https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →
• In metatag_ai.routing.yml you can remove options with _admin_route: TRUE, because everything is behind /admin path is using administration theme
• In metatag_ai.routing.yml you are using permissions but they are not declared, do this in newly created metatag_ai.permissions.yml file. In current situation, the admin can only use this module
• In info.yml file you can't use package: Custom, rename it proper
• In composer.json you should rename your module to "drupal/metatag_ai", because your package is on Drupal repository. "type" is not custom module but contrib now so "type" should be "drupal-module" also licence is required and in info.yml you require module metatag so in require section add this module (in composer.json). Remove empty require-dev. More info how to write proper composer.json here - https://www.drupal.org/docs/develop/using-composer/add-a-composerjson-file →
• In .module file i suggest to add types in functions arguments and return types. In metatag_ai_form_alter description beyond hook implementation you should add your description why you used this hook
• In src folders files you should also add return and arguments types of functions, your code will more stable then. Properties also should be typed.
• In GenerateMetatag.php use Dependency Injection. Whenever there is OOP code you have to always use DI, more info here https://www.drupal.org/docs/drupal-apis/services-and-dependency-injectio... →
• In GenerateMetatag.php:90 you are using service. Inject this through DI and use as property eg.: $this->state_config
• Remove validateForm() from MetatagAISettingsForm.php this method is not required and it's empty.
• In MetatagAIConfigForm:95 you have translated string, but it is broke after word "the", avoid this, because in translation page you will see /n i think and it is not correct.

Also fix those errors from phpcs

FILE: /opt/drupal/web/modules/custom/metatag_ai/README.txt
----------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------
  1 | WARNING | Line exceeds 80 characters; contains 335 characters
  7 | WARNING | Line exceeds 80 characters; contains 111 characters
  8 | WARNING | Line exceeds 80 characters; contains 120 characters
 10 | WARNING | Line exceeds 80 characters; contains 95 characters
 11 | WARNING | Line exceeds 80 characters; contains 85 characters
 16 | WARNING | Line exceeds 80 characters; contains 162 characters
 18 | WARNING | Line exceeds 80 characters; contains 177 characters
----------------------------------------------------------------------


FILE: /opt/drupal/web/modules/custom/metatag_ai/metatag_ai.info.yml
-------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------
 1 | WARNING | Remove "project" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
 1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
-------------------------------------------------------------------------------------------------------------

A lot of work before You, but i wish luck with that ;)

Thank all of You for your reviews and helpful tips, i'll definitely use them in the future! :)

I'm still wondering how many more reviews I need or how much time it will take to process this application?

I read the entries about Bonus program, but i see that there is no many active applications and i have reviews here, so i don't know is this necessary?

Thank You a lot @andrei.vesterli for deep review and good word! It's means to me a lot.

I analyzed your review and i made some fixes, also i have some questions:

• Readme is renamed and have updated syntax, thx for that tip i never saw anything telling about readme.
• You're right, i shouldn't support gin theme, so i removed related code with this.
• Purpose of re_mgr.api.php file is to inform developers that they can use this hook in their own modules if needed. It's common practice to keep information about all modules hooks in this place.
• Module is supporting PHP from 8.1 so it's compatible only with D10. It's very possible that it will run on PHP 7.4 even and D9, but it is a fresh module, not many people will use it on older D9 sites, also its not hard to upgrade to D10. Last is that i'm alone and i want to support only fresh D10.
• Thank You for code optimize tips, i included them on new module version. I love clean code so this will help me a lot in the future!
• I did not changed properties definitions because i really don't like this kind of syntax. Is this necessary to use it? If it's necessary please give me a link about this because i'm confused why to use them if we have typed properties?
• I added docs to the presentation block twig file.

@harishh look at new version i've added help pages to all submodules.

But $this->t() can't be used in static methods like this ::baseFieldDefinitions()

@vishal.kadam please check new commit i fixed the phpcs errors. Thank You also for tip because i didn't know that phpcs can check also .yml files. Also i have no idea why phpcs didn't showed me those css files before i started this application.

What next guys? ;)

Thank You @apaderno for tip!

Yes, but according to https://www.drupal.org/node/1011698#s-prerequisites → i have to be author of at least most of this code. The code is 100% mine but commits meta shows different email so as You mention they are not associated to my account.

I'll be watch out for this in the future, in any commits, but question is - Will this code be recognized as mine within this application or do I need to add this code as mine again? For example, adding a commit where I remove all the code, then adding a commit where I add it again with my email, is this the correct way? I don't want to mess things up..

Additionally, I wanted to ask - if the code is not assigned to me, will I have any difficulties in the future somewhere on Drupal or will I not get any credits from it even if I am projects maintainer? Maybe it's better on the start reassigned the code to me and how to do it proper?

Thanks @apaderno for pointing that. Of course i can change email and name in local Git settings, but what should i do with my projects repo now? I can't push full code again cause it will not see any changes and it will not update users data.

Mattixonix → created an issue.

Mattixonix → created an issue.