Page MenuHomePhabricator

show merged commits in herald emails for merge commits
ClosedPublic

Authored by fabe on May 25 2015, 10:31 AM.

Details

Summary

Includes a new Block in Herad emails which tells the user about which commits were merged in a merge commit
Otherwise the email would just say "Merge branch XYZ". Ref T8295

Test Plan

imported various commits (and merges) and watched resulting herald emails for all of them

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fabe retitled this revision from to show merged commits in herald emails for merge commits.May 25 2015, 10:31 AM
fabe updated this object.
fabe edited the test plan for this revision. (Show Details)
fabe updated this revision to Diff 31326.
fabe edited edge metadata.May 25 2015, 10:59 AM
fabe updated this revision to Diff 31330.
  • add diffusion.fields config and disable mergedcommits by default & celerity map fix
epriestley requested changes to this revision.

Some minor stuff inline, this generally looks good to me.

src/applications/repository/customfield/PhabricatorCommitMergedCommitsField.php
33

This should be the actual viewer -- I think $this->getViewer() will work.

34

I think this is no longer used for anything and has no effect (we determine it automatically in all cases, now).

40

Using ConduitCall directly (as in PhabricatorCommitBranchesField) might be a little simpler than building a DiffusionRequest.

41

Use real viewer.

52–53

Use pht().

57–61

Maybe simpler with foreach ($merges as $merge).

68–69

Maybe put $ex->getMessage() in the mail body? So we end up with something like:

MERGED COMMITS
Error generating merged commits: blah blah blah useful error message.

That seems more obvious/helpful than just dumping it to the webserver error log.

This revision now requires changes to proceed.May 25 2015, 11:22 AM
fabe edited edge metadata.May 25 2015, 11:47 AM
fabe updated this revision to Diff 31340.
  • fix inlines
epriestley edited edge metadata.May 25 2015, 11:48 AM
epriestley accepted this revision.
This revision is now accepted and ready to land.May 25 2015, 11:48 AM
This revision was automatically updated to reflect the committed changes.