Module review

For me "b" seems like a feature not a bug. If I want to put 100% of my votes on a single issue I think I should be able to. The voting widget makes complete sense to me as it is (screenshot attached where I've voted for an issue 3 times and have 2 votes left).

Thanks for the review. My replies...

a) That's by design. We discussed this in depth with Acquia when they were sponsoring this module. You should manually reclaim your votes. Otherwise, all votes on an issue can be lost by a careless user setting it to "closed" prematurely, etc. Plus, it gives you the nice feeling of visiting the "issues I voted on page" and seeing a bunch of things marked "closed" as a sense of accomplishment.

b) Again, by design. If I only care about 1 issue with all my might, I should be able to indicate that. This isn't just a "i'll sprinkle votes wherever I feel like it so they don't mean anything" feature, this is a way for users to add weighting to the issues they most care about. b.2) "issue votes by user table" should be a view, but we ran out of time/funding for that, and Acquia didn't think it was important.

c) I don't think that necessarily helps performance, and it defeats the purpose of (a).

d) see b.2 above. ;) Yes, this should be a view. A patch to do this would be welcome (or funding -- I'd happily do it myself)

e) "Code clean-up in some places required" isn't helpful. ;) I was pretty careful about this when reviewing hunmonk's original version of the code. Can you please be specific?

"for performance reasons, Panels + Views integration should probably be moved into a separate sub-module " -- they're already in conditionally loaded .inc files. The only cost if you don't have either module enabled is parsing the following code:

/**
 * Implementation of hook_views_default_views.
 * @see _project_issue_voting_views_default_views
 */
function project_issue_voting_views_default_views() {
  require_once(drupal_get_path('module', 'project_issue_voting') .'/project_issue_voting.views.inc');
  return _project_issue_voting_views_default_views();
}

/**
 * Implementation of hook_panels_default_panel_views.
 * @see _project_issue_voting_default_panel_views()
 */
function project_issue_voting_default_panel_views() {
  require_once(drupal_get_path('module', 'project_issue_voting') .'/project_issue_voting.panels.inc');
  return _project_issue_voting_default_panel_views();
}

/**
 * Implementation of hook_panels_default_panel_views.
 * @see _project_issue_voting_panels_default_panel_pages()
 */
function project_issue_voting_default_panel_pages() {
  require_once(drupal_get_path('module', 'project_issue_voting') .'/project_issue_voting.panels.inc');
  return _project_issue_voting_default_panel_pages();
}

The cost of putting these in a whole separate module far outweighs the cost of parsing these 3 trivial functions.

Let's move discussion of user/N/project-issue-votes over to #328689: Convert user/N/project-issue-votes into a view.

Seems like the only other thing in here is the vague "minor code cleanup" mentioned in (E) of the original issue, and I don't know what that means, so I'm setting this to NMI.

Cool, thanks, that's more helpful. I'm laughing because more than one of these hunks are things I pointed out to hunmonk when I first reviewed his draft of the module, but I guess they never made it into the final code. ;)

A) Attached is a file containing the patch hunks I didn't apply -- I don't think they help readability of the code, and they're not mandated by the coding standards.

B) I didn't apply the patches against views.inc and panels.inc -- until panels and views export code that conforms to our coding standards, I think it's better to leave this stuff as close to the autogenerated output as possible, so that if you modify something, re-export it, and put it in place, you just the diff of what you changed, not all the code style changes, too.

C) In a few cases, I re-worded the first line of the PHPdoc to stay within 80 chars but still be a single line on its own as a summary for IDE users, etc.

Otherwise, I applied all the hunks, did some light testing, and committed to HEAD. Thanks.