On 2017-03-13 4:38 PM, Mike Connor wrote:
> On Mon, Mar 13, 2017 at 4:35 PM, Bobby Holley wrote:
>> On Mon, Mar 13, 2017 at 1:24 PM, Lawrence Mandel
>>> 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. Keeping the review system in sync with what
>>> (regardless of the review requirements) will make it easier to automate a
>>> repo audit and reduce the time that our reviewers need to spend looking
>>> code changes in the audit scenario. Any concerns with making it a
>>> requirement that the final patch/diff is documented in the bug/review
>>> rather than landing directly?
>> Submitting the final patches to two places instead of one seems like
>> busywork to me, and I don't do it (even though some do). I don't know what
>> it buys us, given that pulsebot posts the hashes of the pushed changes in
>> the bug.
>> So I would object to this.
> I'd agree that posting a bug to two places is a non-starter. That's a
> problem we can and should solve with automation. It should be possible to
> commit to somewhere, have it show up in Bugzilla, and have it land where it
> needs to go.
I feel like I'm starting to reply to every message in this thread with
"rebases, rebases, rebases". ;-)
More seriously, what Lawrence is asking for is simply just impossible.
You can't know what the final patch you will push will be before you
push it due to the fact that we prohibit pushing more than one head, so
you may have to do an unlimited number of rebases before you actually
succeed in landing your patch.
Once your patch has landed, then pulsebot already sends a link to the
landed commit to the bug, and that is what has to be counted on as the
actual source of truth as to what needs to landed. The fact that
MozReview is incapable of reflecting that in its UI is just a deficiency
of the review tool which we could fix. I use Phabricator for
contributing to LLVM, and there as soon as I manage to push the final
version of my patch to their SVN repo, Phabricator UI links my review
page to the real SVN revision landed (example:
"Was committed in r260265, but reverted in r260536.")