Election

Please check all coding standard related issue. Here is the link where you can check the list of error/warning.
http://ventral.org/pareview/httpgitdrupalorgsandboxpjcdawkins1167066git-...

election.module
-------------------
line 372:

if (!empty($type['post key'])) {
      $post_key = $type['post key'];
      $post_name = _election_get_posts_name($type_mn);
      $post_name_plural = _election_get_posts_name($type_mn, TRUE);
      $items['admin/config/election/' . $type_mn . '/' . $post_key] = array(

I think what you are trying to accomplish here can be done using arguments, e.g. $items['admin/config/election/%type_mn/%post_key']

line 315

'title' => 'Elections',
    'description' => 'Administer elections and referendums.',

all the titles and descriptions would have to be wrapped in t()

correct me if I am wrong but do we really need function election_menu_alter(&$items) { ? is it not possible to achieve the functionality in function election_menu(&$items) { itself ?

Manual Review:
1. First I download your module and try to install it gives me an error "date_popup" is required. I try to download "date_popup" via drush but its failed. Then I'm trying to googling and found date_popup comes under date module. So It should require to add date module as a dependency instead of date_popup
2. After installing this I have found "Add new referendum" and "Add new STV election" which have some voting status configuration but not able to identify what it actually does? Then I follow your README.txt and found its dependent on other module which are "election_post" and "election_candidate".
3. So its required more detail documentation on how to use.
Electronics Review:
Its have lots of coding-standard issue which people already mentioned above.

Hi pjcdawkins,

Manual Review:

Drupal API

• I suggest that all instances of require() and require_once() be replaced with the Drupal API function: module_load_include()
• Functions that process output or utilize the Drupal theme API may need to be moved into a new file: perhaps election.theme.inc - for better code segmentation.
• For ease of edit-ability - it is encouraged that you use the Drupal Theme API wherever possible: for instance in the table being generated in election_vote_stv_vote_form()
• any hardcoded <a> links should be replaced with the l() function wherever possible.
• vote.js might be able to make use of the Drupal Behaviors functionality.
• vote.min.js may not be needed. All script passed through Drupal's #attached functionality will be subsequently aggregated and Gzip'd by Drupal's caching engine. It's up to you, but personally: for such a small amount of Javascript I would just include the source.
• It is suggested that Drupal's Database Abstraction Layer be used wherever possible. Notably in election_statistics_preprocess_election_statistics() in election_statistics.module

Entity API

The functions near line 166 of election.module can probably greater utilize the functions available in the entity API module.

I would suggest that election_load_multiple() on line 183 of election.module be removed, and replaced with just a regular entity_load().

While election_load() on line 166 of election.module seems to be replicating functionality similar to entity_load_single().

I would suggest that both of these methods be replaced with the methods already available in the entity module.

I have had a further look and have noticed that the module defines quite a few functions that seem to replicate functionality already available in the entity API module. I would suggest that you review your implementations of election_delete() and election_save() and any other functions that replicate functionality present in the entity API module and replace where applicable.

Great job on the module BTW, i'm sure it will see a lot of use upon release. Cheers, Charlie.

Thanks for posting this module!

Last year when we ran the Drupal Association elections we used the Decisions module, http://drupal.org/project/decisions (D6 only). Did you look at Decisions? How does it compare to what you've done? Are there points of potential convergence?

To do a recent online election I actually used the Webform module in place of Decisions based on the necessity to have write-in candidates. With the 'Select or other' add-on, that's doable in Webforms...

Eric

Minor changes.

While this module overlaps with what Decisions tries to do, this looks to be one of the cases where multiple models and a fresh approach make good sense. Possible some Decisions plugins could be rewritten for Election, or they could adopt a common plugin format.

I've had a look through the code. I posted some minor issues, none of which should hold up approval of the module. I didn't find time to review all code--there is a lot of it! But from what I did review, I am very impressed with this as a first substantial module contribution.

The module makes excellent use of both core and contrib APIs. It is well abstracted. And it addresses a clear use case that needs attention.

If this is an example of initial contributions these days, we've come a long way since I posted my first module is all I can say ;)

manual review:

1. "See: http://api.drupal.org/api/drupal/includes--common.inc/function/entity_uri/7.": do not hardcode such links, just use "@see entity_uri()". Docs: http://drupal.org/node/1354#general
2. The automated review shows that some comment lines exceed 80 characters.
3. election_menu_alter(): why do you need that hook? Please add a comment.
4. election_entity_update(): if you are only targeting your own entity type you should use hook_ENTITY_TYPE_update(). Same for the delete and insert hook.

Otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

The election_candidate_download_form_submit() function currently does 3 separate tasks: parses the choice of properties to output, queries the DB to build the CSV output, and provides output (and http headers). Ideally these should be split into separate APIs so that other modules can make use of them.

Generally, exit() shouldn't be called; drupal_exit() should be used instead.

All instances of 'und' (e.g. $value['und'][0]['safe_value']) should be replaced with the language constant LANGUAGE_NONE.

The menu-alter function election_candidate_menu_alter() could use a comment to explain why it's being used (rather than standard hook_menu, for example).

There's a typo in election_candidate_menu_local_tasks_alter(), with the a call to user_access('bypass nomination schedule'): there are 2 spaces after 'bypass', so this check will fail for everyone except user 1.

The election module implements hook_hook_info() for hook_election_type_info(). The hook's group param allows the hook to be implemented in modulename.election.inc, and these files will be automatically included on demand, so the implementations of that hook within the election modules should be moved to new foo.election.inc files too.

So a couple of minor nit-picking issues, but certainly nothing to hold this module back: +1 for RTBC.

I didn't have a look at all of the code, but where I looked at it's pretty clean and I don't thing here are major issues.

So I went over to test the module

• After installing the submodules I had to re-clear the cache, so the admin items actually appeared. strange
• All requirements of libraries should also be validated on the status reports page by hook_requirements()
• When editing and creating positions, you don't check whether the "number of vacancies" really is a number

Beside this I was not able to find any major issues regarding security,
therefore

Thanks for your contribution!

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:

• 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 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.

Once this is mature enough, we can use it on d.o for voting on the location of the next DrupalCon! :P