Page MenuHomePhabricator
Differential D9609

Show repository in Differential emails
ClosedPublic
Actions

Authored by joshuaspence on Jun 17 2014, 10:21 PM.
Tags
None
Referenced Files
F14375139: D9609.id23054.diff
Fri, Dec 20, 9:57 PM
Unknown Object (File)
Thu, Dec 19, 8:07 AM
Unknown Object (File)
Thu, Dec 19, 7:41 AM
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
View All Files
Subscribers
epriestley
Korvin
• young_hwi
zacchiro

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
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

joshuaspence updated this revision to Diff 23033.Jun 17 2014, 10:21 PM
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.
joshuaspence added a task: T5137: Make emails easier to filter.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJun 17 2014, 10:21 PM
Herald added subscribers: Korvin, epriestley. · View Herald Transcript
Harbormaster completed remote builds in B1194: Diff 23033.Jun 17 2014, 10:22 PM
epriestley edited edge metadata.Jun 17 2014, 10:29 PM

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 updated this revision to Diff 23054.Jun 18 2014, 1:05 AM
joshuaspence edited edge metadata.
  • Use the revision's repository.
  • Repository name instead of PHID.
joshuaspence edited the test plan for this revision. (Show Details)Jun 18 2014, 1:06 AM
Harbormaster completed remote builds in B1208: Diff 23054.Jun 18 2014, 1:06 AM
joshuaspence added a comment.Jun 18 2014, 1:07 AM

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.

• young_hwi added a subscriber: • young_hwi.Jun 18 2014, 1:43 AM
This comment was removed by • young_hwi.
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 requested changes to this revision.Jun 18 2014, 12:59 PM
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
joshuaspence updated this revision to Diff 23069.Jun 18 2014, 3:22 PM
joshuaspence edited edge metadata.

Add monogram

Harbormaster completed remote builds in B1220: Diff 23069.Jun 18 2014, 3:23 PM
epriestley accepted this revision.Jun 18 2014, 3:25 PM
epriestley edited edge metadata.

Works fine locally.

This revision is now accepted and ready to land.Jun 18 2014, 3:25 PM
joshuaspence added a comment.Jun 18 2014, 3:26 PM
In D9609#17, @epriestley wrote:

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.

epriestley added a comment.Jun 18 2014, 3:35 PM

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

joshuaspence updated this revision to Diff 23071.Jun 18 2014, 4:10 PM
joshuaspence edited edge metadata.

Only send email on diff update.

Harbormaster completed remote builds in B1221: Diff 23071.Jun 18 2014, 4:11 PM
epriestley added a comment.Jun 18 2014, 4:35 PM

Looks good to me.

joshuaspence closed this revision.Jun 18 2014, 4:36 PM
joshuaspence updated this revision to Diff 23074.

Closed by commit rPe0ca39f6a1fa (authored by @joshuaspence).

zacchiro added a subscriber: zacchiro.Oct 1 2018, 7:30 PM
This comment was removed by zacchiro.
epriestley added a comment.Oct 1 2018, 7:32 PM

repository.png (505×1 px, 89 KB)

????????

zacchiro added a comment.Oct 1 2018, 7:34 PM

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

zacchiro mentioned this in D18636: Add the repository name to revisions in Differential list view..Oct 1 2018, 7:39 PM

Revision Contents
Changeset List

PathSize
src/
applications/
differential/
customfield/
22 lines

Diff 23074

View Options

src/applications/differential/customfield/DifferentialRepositoryField.php

Loading...
Phabricator · Built by Phacility