[D8] Chatbot API

Hey @gambry,

i am able to see the some error Pareviw of your project.

The chatbot_api_alexa.module does not implement hook_help()
DrupalPractice has found some issues with your code, but could be false positives.
Coder Sniffer has found some issues with your code (please check the Drupal coding standards).

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

Automated Review

Pareview details: https://pareview.sh/pareview/https-git.drupal.org-project-chatbot_api.gi...

Review of the 8.x-1.x branch (commit 973ed2d):

• README.md or README.txt is missing, see the guidelines for in-project documentation.
• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• DrupalPractice has found some issues with your code, but could be false positives.

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

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template. Is missing see pareview

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements

Coding style & Drupal API usage

(*) Coder Sniffer and DrupalPractice in pareview

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

#8 doesn't report any coding style issue. Which coding style issue would break the API?

I agree with kiamlaluno. I can't find where the API would break, if you change the code to match with Drupal coding standards.

Review of the 8.x-1.x branch (commit ba50156):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: ..._temp/modules/chatbot_api_entities/src/Form/EntityCollectionForm.php
  --------------------------------------------------------------------------
  FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
  --------------------------------------------------------------------------
   344 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   345 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   346 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   347 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   348 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
  --------------------------------------------------------------------------
  
  Time: 3.89 secs; Memory: 6Mb
  

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

FILE: ...mp/modules/chatbot_api_entities/src/Plugin/QueryHandlerInterface.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 27 | ERROR | Arguments with default values must be at the end of the
    |       | argument list
--------------------------------------------------------------------------


FILE: ...ntities/src/Plugin/ChatbotApiEntities/QueryHandler/DefaultEntity.php
--------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------
  5 | WARNING | [x] Unused use statement
 37 | ERROR   | [ ] Arguments with default values must be at the end of
    |         |     the argument list
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...i_entities/tests/src/Functional/ChatbotApiEntitiesFunctionalTest.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 48 | ERROR | Public method name
    |       | "ChatbotApiEntitiesFunctionalTest::testAdminUI" is not in
    |       | lowerCamel format
--------------------------------------------------------------------------


FILE: ...sh/pareview_temp/src/Command/Generate/PluginChatbotIntentCommand.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 16 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Time: 5.64 secs; Memory: 8Mb

There are still methods where the arguments with default values aren't the last in the argument list. That is not allowed from PHP.

public function query(EntityTypeManagerInterface $entityTypeManager, array $existing = [], EntityCollectionInterface $collection) {
  $entity_type_id = $collection->getCollectionEntityTypeId();
  $entity_type = $entityTypeManager->getDefinition($entity_type_id);
  $query = $this->getQuery($entityTypeManager, $collection, $entity_type);
  return $existing + $entityTypeManager->getStorage($entity_type_id)->loadMultiple($query->execute());
}

If the module is already used, create a new branch, don't create new tags, and fix the code there.

(That was the wrong status.)

I see no issues in branch 2.x

I am going to take the final step in 4 hours (less or more).

Thank you for your contribution!

I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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.