[LLVMdev] RFC: Code Ownership

[Sean Silva]

On Mon, Nov 12, 2012 at 3:19 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
> On Mon, Nov 12, 2012 at 10:40 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> Phabricator provides a way to set up notifications for changes to
>> certain parts of the codebase & you can then "accept" those changes
>> which is probably the right way to do this, but I haven't tried the
>> workflow myself.
>>
>> - David
>
> I currently use this and find it works quite well. The only issues
> with it that I've found are that everyone has to be registered and
> have the same username as svn for "raise concern" to alert them. The
> other is that you still get an audit even if the patch was approved
> pre-commit.

Yeah, for me, the fact that it doesn't alert them really detracts from
the usefulness of an audit feature; it's like speaking into a void.
It's also incredibly annoying because most of the patches I have
either already looked at during pre-commit review or have seen the
automated commit mail on -commits and accepting a commit on
Phabricator is at least 3 clicks and a long scroll.

> The first issue is already being worked on. The second isn't really a
> generally solvable problem unless you include the phabricator
> Differential number in the commit message, as quite a few reviews end
> with "change x and commit".

This actually doesn't sound hard at all; you can just compare the
commit patch with the differential patch(es). I'm actually currently
working on a project which is much more general in that it matches up
mailing list code-review and RFC/design threads for a feature with
commits implementing the feature; one of the things that it can look
at is attached patches and compare with a commit's patch.

-- Sean Silva

On Mon, Nov 12, 2012 at 3:19 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:
> On Mon, Nov 12, 2012 at 10:40 AM, David Blaikie <dblaikie at gmail.com> wrote:
>> Phabricator provides a way to set up notifications for changes to
>> certain parts of the codebase & you can then "accept" those changes
>> which is probably the right way to do this, but I haven't tried the
>> workflow myself.
>>
>> - David
>
> I currently use this and find it works quite well. The only issues
> with it that I've found are that everyone has to be registered and
> have the same username as svn for "raise concern" to alert them. The
> other is that you still get an audit even if the patch was approved
> pre-commit.
>
> The first issue is already being worked on. The second isn't really a
> generally solvable problem unless you include the phabricator
> Differential number in the commit message, as quite a few reviews end
> with "change x and commit".
>
> - Michael Spencer
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev