Quickly looked over the code.

43       $blocks['subject'] = t('Notes for User <span id="user-name">') . $user->name . t('</span>');

Use placeholders for dynamics. You can use the %placeholder and it will automatically be highlighed, so you don't need that custom span. Like this:

 t('Notes for User %name', array('%name' => $user->name));
  56   $items['note-Add'] = array(
  57     'title' => "Add a Personal Note",
  58     'type' => MENU_NORMAL_ITEM,
  59     'menu_name' => 'primary-links',
  60     'page callback' => 'personal_notes_note_add',
  61     'access arguments' => array('add a note'),
  62   );

Mixing case in URL's is very uncommon, using - too, what about using note/add instead? Also, I don't think you should add it to the primary-links menu, the site owner can always do that himself if he wants to.

  88 /**
  89  * Add a Personal Note page.
  90  * This creates the form for adding a personal note.
  91  *
  92  * @return
  93  *  Form HTML.
  94  */
  95 function personal_notes_note_add() {
  96   return drupal_get_form('personal_notes_note_add_note');
  97 }

This is not necessary, instead, simply use 'drupal_get_form' as the page callback in your menu item and 'personal_notes_note_add_note as the first page argument.

 149   drupal_set_message(t("Your new note <b>$title</b> has been created."));

Same here, you need to use placeholders to make it actually translatable and avoid XSS issues.

 211       $msgbfr .= sprintf("<br />deleting note <b>%s</b> by user %d created on %d\n",
 212                        $fields[title], $user->uid, $element);

And here you should use t() too

 231   $query = "SELECT title, note, created FROM {personal_notes_notes} WHERE uid=%d
 232     ORDER BY created LIMIT %d;";
 233   $results = db_query($query, $user->uid, $num_items);

Use db_query_range() instead for limiting the results, directly using LIMIT isn't portable (does not work on PostgreSQL like this)

<div class="personal_notes">
   2   <div class="title"><?php print $title ?></div>
   3   <div class="note"><?php print $note ?></div>
   4   <hr class="sep">
   5 </div>

You need to protect against XSS with check_plain(), either here or when you before passing it to the theme function (But always only before displaying, not before saving).

 ;$Id: Personal Notes,v 1.0 2011/04/29 marty $

Not necessary anymore, git doesn't use that. Just remove it.

Comments

marty’s picture

Thanks for the review Berdir, very educational for me. I've made all the changes and done the commits - they're on http://drupal.org/commitlog/commit/22148/50da5915bdc472f5ce2eb9e0bfa68d3....

marty’s picture

Assigned: Unassigned » marty
Status: Active » Fixed

Fixed now with the commit I did last night

Status: Fixed » Closed (fixed)

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