Phabricator + 'Reviewed by' [was Re: svn commit: r278472 - in head/sys: netinet netinet6]

[Bryan Drewery]

On 2/14/2015 9:17 AM, Steven Hartland wrote:
> 
> On 13/02/2015 23:56, Bryan Drewery wrote:
>> On 2/9/2015 3:45 PM, Bjoern A. Zeeb wrote:
>>>>   Commented upon by hiren and sbruno
>>>>   See Phabricator D1777 for more details.
>>>>
>>>>   Commented upon by hiren and sbruno
>>>>   Reviewed by:    adrian, jhb and bz
>>> I have not reviewed this;  as a matter of fact you are aware that I
>>> still wanted to do that.
>>>
>> Something about Phabricator is not jiving with our commit terminology.
>> This has happened before as well with other commits. I'm sure everyone
>> is good-intentioned as well.
>>
>> There's not 1 person on D1777 who has 'accepted' it. That is what
>> warrants a 'Reviewed by' to me.
>>
>> It's clear to me, but seems unclear to others. I really think the
>> reviewer list needs to be split up. Rather than using icons, use
>> separate lists. Reviewers requested: accepted: commented: changes
>> requested:.
> I don't think it needs to be split up, that feels unnecessary, if
> someone hasn't accepted it then they haven't review it period IMO.

Yes I too think it's obvious, yet I've seen at least 2 commits where the
reviewed by line was essentially a lie. It's in SVN forever now with
those names stamped as reviewers.

-- 
Regards,
Bryan Drewery

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20150214/e124360c/attachment.sig>