The module provides a webform CPF field to be used in webforms. (CPF is a Brazilian ID.)

Project link

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

Git instructions

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

PAReview checklist

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

Comments

thiagogomesverissimo created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: feature » module
Issue summary: View changes

Thank you for applying! I added the Git instructions for non-maintainer users and the PAReview checklist link. Reviewers will check the project and post comments to list what should be changed.

If you haven't done it, yet, please check the PAReview report and fix what needs to be fixed. There could be some false positives; verify that what reported is correct, before making any change.

jasonawant’s picture

jasonawant
he/him
Primary language English
Location New Orleans, USA
commented

Here's a single comment apart from the PAReview: https://pareview.sh/pareview/https-git.drupal.org-project-webform_cpf.git

The *.info.yml file lists three dependencies, but the README only lists one: https://git.drupalcode.org/project/webform_cpf#requirements

Also, while it's likely apart to webform users, the README could include a configuration/usage section about this.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work
thiagogomesverissimo’s picture

thiagogomesverissimo commented
Status: Needs work » Needs review

Guys,

Thanks you for your revision!

I adapted the code as suggested and the following was changed:

  • master branch deleted
  • required modules list fixed in: composer.json, webform_cpf.info.yml and README.md
  • all required section in README.md was created
  • dependencies section in webform_cpf.info.yml was prefixed with the project name
  • All warnings from phpcs was fixed, based on report from command: phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml webform_cpf
rksyravi’s picture

rksyravi
Primary language Hindi
Location New Delhi, 🇮🇳
commented

In these files you have used services /webform_cpf/src/Plugin/WebformElement/WebformCpf.php and /webform_cpf/src/Element/WebformCpf.php
Please use dependency injection.

vernit’s picture

vernit
he/him
Primary language English
Location India
commented
Status: Needs review » Needs work

Please refer the #6 to work on it i.e. Please use dependency injection.

thiagogomesverissimo’s picture

thiagogomesverissimo commented
Status: Needs work » Needs review

Code changed from services to dependency injection, as asked in #6.

Thanks again for yours reviews!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community

Thanks for you contribution!

WebformCpf::form(): looks like this method does not do anything and can be removed?

Otherwise looks good to me!

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
Status: Reviewed & tested by the community » 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.

thiagogomesverissimo’s picture

thiagogomesverissimo commented

thanks guys!

Status: Fixed » Closed (fixed)

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