[D8] Seo Keyword Links

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Automated Review

http://pareview.sh/pareview/httpsgitdrupalorgsandboxleandro7132709361git...

1. No Readme
2. Some codestyle errors

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.

Code long/complex enough for review

[No: Does not follow] the guidelines for project length and complexity. Just 2 functions and 82 lines of code in .module file

Secure code

[Yes: Meets the security requirements.]

Coding style & Drupal API usage

1. Notice after module is enabled: Notice: Undefined index: #node in seo_keyword_links_entity_view_alter() (line 51 of modules/contrib/seo_keyword_links/seo_keyword_links.module)
2. It would be more convinient if list of links is configured via admin panel.
3. Allow administrator to specify which node types AND fields he/she wants the module works on via admin panel.
4. I added the link in CKeditor <a href="http://google.com">Ono</a>, but after saving I got <a href="http://google.com"></a><a href="http://ono.es">Ono</a> Don't think this is correct behaviour.
5. In seo_keyword_links.info.yml name: seo_keyword_links. Please replace the name with more readable without underscores since this will be displayed in the list of modules.

This review uses the Project Application Review Template.

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

Automated Review

http://pareview.sh/pareview/httpsgitdrupalorgsandboxleandro7132709361git-8x-1x

Looks like there are some minor formatting issues in the SimpleTest output, probably worth looking into.

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

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

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

A few issues here:

1. Agree with #5, it'd be great to configure this via the admin panel. Even though you prefer text config, general users might like to use the admin interface. Plus, this means you won't have to do a code push every time you want to add a word.
2. Saw that updated the info.yml file with a more readable name, nice!
3. No longer seeing the notice after installing, great work!
4. *Re: point 4 in #5 above, it's be great to detect whether the word is already wrapped in a link. Additionally, it looks now like it replaces the work text anywhere it appears. So, if a user wrote the word that contains another word, it will link the partial portion of that word. For example, if I define `test` as a link, the word `testing` would be rendered as `testing`. It be nice limit the find/replace to full words.

This review uses the Project Application Review Template.

Hello leandro713,

I checked your module, great idea, however there are some styling issues and recommendations that you may be interested in :

FILE: ...s/pareviewsh/pareview_temp/src/Tests/SeoKeywordLinksTestTestCase.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
36 | WARNING | Unused variable $title.
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/src/Form/SeoSettingsForm.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
--------------------------------------------------------------------------
40 | WARNING | #description values usually have to run through t() for
| | translation
48 | WARNING | #description values usually have to run through t() for
| | translation
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/seo_keyword_links.routing.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
8 | WARNING | The administration page callback should probably use
| | "administer site configuration" - which implies the user
| | can change something - rather than "access administration
| | pages" which is about viewing but not changing
| | configurations.
--------------------------------------------------------------------------

And some coding style :

FILE: ...s/pareviewsh/pareview_temp/src/Tests/SeoKeywordLinksTestTestCase.php
--------------------------------------------------------------------------
FOUND 7 ERRORS AND 1 WARNING AFFECTING 8 LINES
--------------------------------------------------------------------------
16 | ERROR | [ ] Doc comment short description must be on a single
| | line, further text should be a separate paragraph
17 | ERROR | [ ] Missing @var tag in member variable comment
19 | ERROR | [ ] Class property $test_nid should use lowerCamel naming
| | without underscores
27 | ERROR | [x] Short array syntax must be used to define arrays
33 | ERROR | [x] Short array syntax must be used to define arrays
37 | ERROR | [x] Short array syntax must be used to define arrays
42 | ERROR | [x] Short array syntax must be used to define arrays
61 | WARNING | [ ] Possible useless method overriding detected
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/src/Form/SeoSettingsForm.php
--------------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
--------------------------------------------------------------------------
57 | WARNING | Possible useless method overriding detected
81 | ERROR | Doc comment is empty
84 | ERROR | Private method name "SeoSettingsForm::_walk_links" is not
| | in lowerCamel format
92 | ERROR | Doc comment is empty
95 | ERROR | Private method name "SeoSettingsForm::_code_links" is not
| | in lowerCamel format
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/seo_keyword_links.module
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
1 | ERROR | [x] The PHP open tag must be followed by exactly one blank
| | line
28 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 8
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

I also think it would be easier for users to select node types from a select or - even better - from a checkbox list, this way the margin of wrong typing the node type is zero.

Good job mate,

Great leandro! did you ship another version ? update the issue settings so we can test the right code ;)

Automated Review

Pareview details: https://pareview.sh/pareview/https-git.drupal.org-sandbox-leandro713-270...

Git errors:

• Git default branch is not set, see the documentation on setting a default branch.

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

• Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
  • The REQUIREMENTS section is missing.
  • The CONFIGURATION section is missing.
• 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

No: Does not follow the guidelines for master branch. Git default branch is not set, see the documentation on setting a default 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. 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

1. (*) see pareview details

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.

If you are still working on this application, you should fix all known problems and set the status to Needs review. (See also the project application workflow.)
Please don't change status of this application if you aren't sure you have time to dedicate to this application, or it will be closed again as won't fix.

I am closing this application due to lack of activity.