Page MenuHomePhabricator

Show repository in Differential emails
ClosedPublic

Authored by joshuaspence on Jun 17 2014, 10:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 15, 12:52 AM
Unknown Object (File)
Fri, Dec 13, 10:02 PM
Unknown Object (File)
Thu, Dec 12, 7:47 AM
Unknown Object (File)
Wed, Dec 11, 6:01 PM
Unknown Object (File)
Wed, Dec 11, 9:49 AM
Unknown Object (File)
Mon, Dec 9, 7:40 PM
Unknown Object (File)
Mon, Dec 9, 3:35 PM
Unknown Object (File)
Mon, Dec 9, 2:41 AM

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Maniphest Tasks
T5137: Make emails easier to filter
Commits
Restricted Diffusion Commit
rPe0ca39f6a1fa: Show repository in Differential emails
Summary

Ref T5137. Listing the repository in Differential emails makes it easy to filter.

Test Plan

Eye-ball it.

Diff Detail

Repository
rP Phabricator
Branch
repo-email
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1208
Build 1208: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Show repository in Differential emails..
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

One inline, seems reasonable otherwise.

src/applications/differential/customfield/DifferentialRepositoryField.php
155–158

I think this should use the revision's getRepositoryPHID(), not the latest diff's.

joshuaspence edited edge metadata.
  • Use the revision's repository.
  • Repository name instead of PHID.

If you really want me to test this properly then I'll give it a shot, but I don't really have outbound mail configured on my dev box.

chad retitled this revision from Show repository in Differential emails. to Show repository in Differential emails.Jun 18 2014, 2:07 AM
chad edited the test plan for this revision. (Show Details)
epriestley edited edge metadata.

Let's do:

$repository->getMonogram().' '.$repository->getName()

...so you get:

REPOSITORY
  rP Phabricator

This is less ambiguous, more consistent, and should be more routable for rules that can't use headers or whatever.

For your use case, is adding this field only on create/update (like "BRANCH" works, I think) reasonable? I don't have strong feelings either way, but that came to mind as worth mentioning.

(I can handle testing this, so don't worry about that.)

This revision now requires changes to proceed.Jun 18 2014, 12:59 PM
epriestley edited edge metadata.

Works fine locally.

This revision is now accepted and ready to land.Jun 18 2014, 3:25 PM

For your use case, is adding this field only on create/update (like "BRANCH" works, I think) reasonable? I don't have strong feelings either way, but that came to mind as worth mentioning.

Yeah, that's reasonable. I'm not entirely sure how to do that.

You can check DifferentialBranchField -- basically if $editor->getDiffUpdateTransaction($xactions) exists.

joshuaspence edited edge metadata.

Only send email on diff update.

joshuaspence updated this revision to Diff 23074.

Closed by commit rPe0ca39f6a1fa (authored by @joshuaspence).

This comment was removed by zacchiro.

I posted the comment to the wrong diff (and hence deleted it immediately afterwards). See D18636. Sorry for the noise.