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
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8295: Show which commits were included in a merge commit in Herald emails
- Commits
- Restricted Diffusion Commit
rPf0d16b3047ff: show merged commits in herald emails for merge commits
imported various commits (and merges) and watched resulting herald emails for all of them
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- mergeCommits
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 6246 Build 6268: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
Some minor stuff inline, this generally looks good to me.
| src/applications/repository/customfield/PhabricatorCommitMergedCommitsField.php | ||
|---|---|---|
| 32 | This should be the actual viewer -- I think $this->getViewer() will work. | |
| 33 | I think this is no longer used for anything and has no effect (we determine it automatically in all cases, now). | |
| 39 | Using ConduitCall directly (as in PhabricatorCommitBranchesField) might be a little simpler than building a DiffusionRequest. | |
| 40 | Use real viewer. | |
| 51–52 | Use pht(). | |
| 56–60 | Maybe simpler with foreach ($merges as $merge). | |
| 67–68 | Maybe put $ex->getMessage() in the mail body? So we end up with something like:
That seems more obvious/helpful than just dumping it to the webserver error log. | |