[Pidgin] #780: Sound event for new email

Pidgin trac at pidgin.im
Mon May 19 12:56:56 EDT 2008


#780: Sound event for new email
----------------------------+-----------------------------------------------
  Reporter:  chrisjames_71  |       Owner:     
      Type:  patch          |      Status:  new
  Priority:  minor          |   Milestone:     
 Component:  pidgin (gtk)   |     Version:  2.0
Resolution:                 |    Keywords:     
   Pending:  0              |  
----------------------------+-----------------------------------------------
Comment (by rekkanoryo):

 Ok, a couple notes then:
   * `//` style comments are unacceptable.  We try to remain C89 (aka ANSI
 C) compliant.  The only type of valid comment in C89 is `/* ... */`.
   * You added a public function `is_new_mail` that is in no way
 namespaced.  It should fit into the function namespace presented in the
 file you added it to.
   * Ideally I'd like to see a signal emitted for new mail and have
 modifications to gtksound.c to attach to the signal instead.  If you
 observe the rest of gtksound.c, you'll see that a number of sound events
 are triggered by libpurple signal emissions.

 There are other issues that I had with this patch, but I don't recall them
 at the moment.

-- 
Ticket URL: <http://developer.pidgin.im/ticket/780#comment:19>
Pidgin <http://pidgin.im>
Pidgin


More information about the Tracker mailing list