1. Sebastian Sebastian
  2. scm-manager
  3. Issues


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 Sebastian 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 Sebastian 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. Tavis Elliott

    I'd be fine with that. Suggestion:

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

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

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

  5. Log in to comment