Issue #410 resolved

Proposed changes to scm-notify-plugin

Anonymous created an issue

Changes my development tam would like to see in the email notification plugin:

  1. Set "from" to be the author of the changeset.
  2. Email per-changeset instead of email per-push.
  3. Change the subject to include: repository, branch, changeset ID (hopefully this is the mercurial hash?)
  4. Include the list of files added/modified/removed in the body of the email.
  5. Include the full unified diff of changes in the body of the email.

I've attached a patch that should handle 1-3, and maybe 4 but not 5.

Proposed TODO's because I'm not sure how in my quick examination: a) be able to configured email per push vs email per changeset b) as stated above, get the unified diff into the email

Setting to major b/c it is a critical issue while we evaluate it for Mercurial use.

Comments (15)

  1. Sebastian Sdorra repo owner

    There are some problems with your patch:

    1. I think changing the from should be configureable, mainly for backward compatibility. For the reason that some guys have create mail filters etc.
    2. If the plugin is configured for one mail per push, the subject line could be very large (think about 1000 or more commits).

    It is possible that you fork the scm-notifiy-plugin and create a pull request. Because it is much easier for me to review patches.

  2. Sebastian Sdorra repo owner

    I've merged your changes, thanks for the patch. But i do not like the look of the E-Mail, when diff and one mail per push is enabled. I think the diff of a changeset should be displayed below the description. What do you think?

  3. Sebastian Sdorra repo owner

    Perhaps we should remove the table completely and use a different structure like:

    (branchname) id - description
    
    diff
    
    (branchname) id - description
    
    next diff
    
  4. Sebastian Sdorra repo owner

    Sorry, but i think you are right we should keep the table with hidden borders and add a new row with colspan for the modifications and another one for the diff.

  5. Tavis Elliott

    I'd be fine with that. Suggestion:

    [branchname] (author) <link>id</link> - commit date
    <pre>description</pre>
    diff summary (+, -, M list of files)
    <pre>
    full diff
    </pre>
    

    Next version it might be nice to be able to specify a template, but that's a bunch of work. :)

  6. Tavis Elliott

    Oh we definitely need to limit the # of lines displayed in the full diff. When I put "full diff" in my suggestion, I didn't mean we should remove the line limit. Definitely needs to remain limited.

  7. Log in to comment