This module implements a format plugin to Address Field module, commonly used with Drupal Commerce. This plugin enable a specific form for addresses in Brazil, according to recomendations of the brazilian postal service company, Correios.

Project page: http://drupal.org/sandbox/gedvan/1586058
Git repo: git clone --recursive --branch master gedvan@git.drupal.org:sandbox/gedvan/1586058.git brazilian_address
Drupal version: 7

Comments

alex.sorokin.v’s picture

alex.sorokin.v commented
Status: Needs review » Needs work

Hi!

I found some ventral issues, please fix: http://ventral.org/pareview/httpgitdrupalorgsandboxgedvan1586058git

-regards

parwan005’s picture

parwan005 commented

Hi,

here is the report from manual review done of your code :-
1) Some of your functions do not have proper commenting. Also when implementing hooks doxygen commenting like Implements hook_requiredhook should be written as you have for hook_menu.
2) At $settings =& $form['#instance']['widget']['settings']... & is of no use, so should be removed.
3) Also file naming convention should be followed for your module related files. I think script.js should be renamed as brazilian_address.js
4) Also why is there this .csv file. I dont see any use of it. Please remove it from repo.

Also i see in your code that proper formatting for function's haven't been followed. You have your code like this :

function funcname
{
// code
}

should be like :

function funcname {
//code here
}

Also make sure to fix your ventral issues.

Thanks
Parwan

gedvan’s picture

gedvan commented
Status: Needs work » Needs review

Thank you guys for the reviews!

I fixed the ventral issues (just a warning remained) and I'm setting the status to "needs review" again, right?
Here is the link: http://ventral.org/pareview/httpgitdrupalorgsandboxgedvan1586058git-7x1x

@parwan005, the CSV file is read in the install function.

Thanks again!
Gedvan

sirup’s picture

sirup commented
Status: Needs review » Reviewed & tested by the community

looks good!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

jthorson’s picture

jthorson commented
Status: Reviewed & tested by the community » Fixed

Took a look through the repo, and had the following comment:

br_address.install
$dir = dirname(__FILE__);: I'd suggest using drupal_get_path() here.

Other than that, looks good, so ...

Thanks for your contribution, gedvan!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, 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.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

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