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
Branch
mergeCommits
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6253
Build 6275: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

fabe retitled this revision from to show merged commits in herald emails for merge commits.
fabe updated this object.
fabe edited the test plan for this revision. (Show Details)
fabe edited edge metadata.
  • add diffusion.fields config and disable mergedcommits by default & celerity map fix
epriestley added a reviewer: epriestley.

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.
  • fix inlines
epriestley edited edge metadata.
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.