Include Differential Revision field in subversion freebsd template

[xmj]

Summary

Patch attached amends devel/subversion's FreeBSD template to include lines for
Differential Revision: that can reference Phabric differentials from subversion commits, and auto-close them.

Can be copied to devel/subversion17 easily; needs more work for
devel/subversion16 whose fbsd template is outdated.

Test Plan

compiled locally;
tested with poudriere on 11.0-CURRENT
http://xmj.me/freebsd/buildlogs/subversion-1.8.9_7.log
and on all other archs through redports
https://redports.org/buildarchive/20140813073721-70548/

[xmj]

I'd love to add Core Team, lev and ohauer as reviewers to this revision, but they don't seem to have Phabric accounts.

are there any workarounds to manually add reviewer's emails?

[mat]

devel/subversion/files/extra-patch-fbsd-template

26

I think it should be before PR:, I quite like having PR and Submitted by next to one another because you don't use PR without using Submitted by.

[emaste]

The canonical version is:

Differential Revision: https://reviews.freebsd.org/D#####

as the last line of the template, as this integrates with Phabricator's auto-closing logic. See for example D529.

[mat]

In D600#7, @emaste wrote:

The canonical version is:

Differential Revision: https://reviews.freebsd.org/D#####

as the last line of the template, as this integrates with Phabricator's auto-closing logic. See for example D529.

Oh, didn't know that was possible, that is a great thing.

[emaste]

Oh, didn't know that was possible, that is a great thing.

Yeah, it's nifty. I tried to get the upstream Phabricator team to implement flexible tags (so we could e.g. use CR: and omit the full url, using just D####), but they were not interested in making that change.

[emaste]

The migration to https://reviews.freebsd.org/ is now done.

[bdrewery]

In D600#13, @emaste wrote:

Oh, didn't know that was possible, that is a great thing.

Yeah, it's nifty. I tried to get the upstream Phabricator team to implement flexible tags (so we could e.g. use CR: and omit the full url, using just D####), but they were not interested in making that change.

Are they willing to take patches? There are a lot of annoying things phabricator does that would be nice to fix.

[emaste]

they willing to take patches? There are a lot of annoying things phabricator does that would be nice to fix.

They're generally quite responsive for bug fixes. I think in general they would be receptive to patches for improvements too.

The specific case here was a design decision they don't agree with, I guess.

[xmj]

Refactor CR: into Differential Revision:,
put as last item, and add tabspacing.

[xmj]

Put Differential Revision: below PR:

[xmj]

Consistent placing of Differential Revision: in prefixes.

[koobs]

+1 on implementation
-1 on naming

Propose: "Differential Code Revision Review in Phabric:"

Upstream not accepting patches (or support trivial customisation) is a fairly weak position, especially given we have template patches for SVN (among other things)

[koobs]

I (mentor) approve the differential revision in syntax only

[mat]

In D600#29, @koobs wrote:

+1 on implementation
-1 on naming

Propose: "Differential Code Revision Review in Phabric:"

Upstream not accepting patches (or support trivial customisation) is a fairly weak position, especially given we have template patches for SVN (among other things)

The problem is not upstream not accepting patches, it's that if you want to change Differential Revision to something else like CR you then have to change arc to use it, and then, our arc would not work with any other phabric, because it would use the wrong magic string.

[koobs]

Fair call.

[xmj]

Committed revision 367920.

[emaste]

I also dislike Differential Revision:, but dislike the idea of maintaining our own patched Phabricator and arc even more. Beyond the arc issue @mat points out, Phabricator uses the tag for automatically closing the review upon commit.

[xmj]

Landed in 367920 and precised by @mat in 368454 -- can't be closed with prior rejects.