Re: The future of commit access policy for core Firefox

Am 14.03.17 um 13:15 schrieb [email protected]:
> On Monday, March 13, 2017 at 8:53:11 PM UTC-4, Nicholas Nethercote wrote:
>> On Tue, Mar 14, 2017 at 7:24 AM, Lawrence Mandel <[email protected]>
>> wrote:
>>>
>>> One issue with r+ with nits that we ran into last year is that the
>>> resulting patch/diff is often committed directly to the repo and not
>>> uploaded back to Bugzilla or MozReview. This makes it difficult to audit
>>> the changes to the repo.
>>
>> I always look at the hg commit link rather than posted patches, precisely
>> because I know that's exactly what landed. Even if the patch author doesn't
>> explicitly change anything after getting review, rebasing can still change
>> the patch.
>>
>> Nick
> 
> I just r-'d a patch last night because a rebase caused it to do unintended things.  While I don't see this everyday, it seems that rebases cause many issues (I see it monthly) and given that data I would say any rebases should be re-reviewed.  How often do patch authors examine their patch in detail after a rebase?  Just looking at the diff can be OK, but seeing the source code in more context can point out the code being added in the wrong place (or in some cases duplicated).
> 
> -Joel
> 

One point on rebases:

autoland, AFAIK, *never* lands the changeset it was asked to land.

It always rebases, and often adjusts the commit message to change the r? 
to r+. (Does it also change the reviewer to the actual reviewer? I'd 
have to check the code.)

I'm pretty sure that at the pace that we're changing the head of our 
repository, we won't get any code landed without rs=foopy on rebases, 
and thus hash changes.

Which also raises an interesting question about how much we can prove 
about the landed patch at the time it lands.

Axel