[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: changes in chord names formatting (1503, 1572) (issue 4981052)

From: Carl Sorensen
Subject: Re: changes in chord names formatting (1503, 1572) (issue 4981052)
Date: Fri, 4 Nov 2011 11:55:42 +0000
User-agent: Microsoft-MacOutlook/

On 11/4/11 5:35 AM, "Adam Spiers" <address@hidden> wrote:

>On Thu, Nov 03, 2011 at 06:28:55PM +0000, Carl Sorensen wrote:
>> On 11/3/11 10:50 AM, "Adam Spiers" <address@hidden>
>> >I originally avoided any pure-whitespace commits, but at the
>> >review of my initial patches, I was told to replace tabs with spaces
>> >in the files I was modifying:
>> >
>> >
>> I can't find the suggestion to replace tabs with spaces in this review
>> string, so I can't comment on the suggestions.

OK, I don't know how I missed that before.
Had I seen that review, I'd have mentioned that .scm indentation is not
yet a settled issue on the GOP.  I'm sorry I didn't catch it earlier.

>> Our policy of testing each commit to ensure that it doesn't break
>> build prevents multiple patches per issue.
>I don't understand why this policy would prevent multiple patches per
>issue, but you probably shouldn't waste time explaining it to me :)

Because the finest granularity we have in the review process is a Rietveld
issue.  So we only check each Rietveld issue to verify that it doesn't
break build.

It would be possible to have a series of three commits in a Rietveld
issue, with the first two commits breaking the build, and the third fixing
the problem.  When this set of three is checked on Rietveld, we get a
clean make, so the push is approved.  If I then push to master as a set of
three commits, two of them break build and therefore mess up git-bisect.

In the case of your chords patch, I looked over each commit carefully and
I'm quite certain that if the build status of the final commit is good,
the build status of all previous commits will be good.  So I'm comfortable
with pushing your chord patches as a set of commits.  But in general, I
don't think that's a good idea.


P.S. Your patch is currently up for review, but with a different issue
number since I'm now the owner

reply via email to

[Prev in Thread] Current Thread [Next in Thread]