When "Differential Revision:" is preceded by a newline or in certain other cases it isn't detected as a field. This means that commits like https://reviews.freebsd.org/rD46733 and https://reviews.freebsd.org/D2116 are not getting recognized and auto-closed. However, commits such as https://reviews.freebsd.org/D2660 are.
When detecting fields, the parser should be more lenient about whitespace and the location of the exact text "Differential Revision:"
Or we need to at least document (in the FreeBSD workflow) how it should be formatted.
LLVM claims (http://llvm.org/docs/Phabricator.html):
When committing an LLVM change that has been reviewed using Phabricator, the convention is for the commit message to end with the line:
Differential Revision: <URL>
where <URL> is the URL for the code review, starting with http://reviews.llvm.org/.
another example of a commit which should have closed a review but did not https://reviews.freebsd.org/rS283923 and https://reviews.freebsd.org/D2019
The underlying issue here is that fields may, in the general case, contain multiple paragraphs, colons, things that look like field headers, and may be line-wrapped. We must parse this as two fields:
Summary: Make some changes to the important UI things: make more of them and make them work better. Lots of people should look at this: it is important! Subscribers: alice, bob, claire, dave, eva, frank, grimes, helena
In particular, "things" is not a field header and "Lots of people should look at this" is not a field header, while both the "Summary" and "Subscribers" fields span across multiple lines.
Thus, when we encounter this message:
Differential Revision: X Some-Custom-Ad-Hoc-Field: Y
...we parse the field value as "X\nSome-Custom-Ad-Hoc-Field: Y", consistent with how other fields work.
D17121 improves our behavior when we encounter these fields by examining only the first line of the value to find a revision.
The preferred approach is to define these custom ad-hoc fields so they are parsed formally, rather than having the parser recover by guessing that they probably aren't field headers. We will provide guidance on this after T9713. DifferentialGitSVNIDCommitMessageField is an example of parsing and discarding a field which you expect to appear in commit messages but do not need Phabricator to interact with.