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

[Carl Sorensen]

On 11/3/11 10:50 AM, "Adam Spiers" <address@hidden> wrote:

>On Thu, Nov 03, 2011 at 04:28:53PM +0000, Graham Percival wrote:
>> On Thu, Nov 03, 2011 at 04:00:41PM +0000, Adam Spiers wrote:
>> > On Thu, Nov 03, 2011 at 11:13:28AM +0000, Peekay Ex wrote:
>> > > One patch per tracker item?
>> > 
>> > I can do that if noone objects to tracker items for patches as trivial
>> > as converting tabs to whitespace?
>> 
>> I'd rather not see those patches.
>> 
>> Hmm, I'm seeing 11 patches?  how hard would it be to do some
>> intelligent rebasing here?  i.e. rebase any programming features /
>> bugfixes into one (or more) patches, rebase the ly file fixes into
>> one (or more) patches, etc.
>
>I always do a lot of patch sculpting via git rebase -i before I deem
>my patches to be good enough to push upstream.  So what you are seeing
>have already been rebased many times in order to obtain commits which
>logically group changes together.  For example, each new feature being
>introduced is accompanied by its corresponding doc tweaks and
>regression tests where relevant.
>
>That said, I *could* squash a few of the smaller patches together.
>But I still think it would be a mistake to squash all these into one
>or two commits, which would then lack a lot of clarity.

As long as each commit can successfully build the regtests, it's good.  My
main concern about the series of commits as one issue is that there is no
check to prove that we can build docs and regtests.

>>Unfortunately, the scheme indentation stuff stalled in August due
>> to a number of factors.  Which was a shame, because scheme
>> indentation is WAY easier than C++ indentation, and also because
>> the indentation script was almost finished.
>
>Understood.  I've already spent WAY more time on all this than
>budgeted, so I'm not sure I can afford to stretch any further for a
>while.  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:
>
>  http://codereview.appspot.com/4981052/

I can't find the suggestion to replace tabs with spaces in this review
string, so I can't comment on the suggestions.

>
>and given my strong aversion to (a) a mix of indentation styles within
>a single file, and (b) commits which mix whitespace changes with real
>coding changes, I thought this was the best way to proceed.  But I
>agree the sensible thing would be to fix all .scm files in one go.

There were actually some comments from senior developers opposing this
plan, which was part of the reason it's never been finished.
>
>> > If Rietveld doesn't support multiple patches per issue then that
>> > sounds like a fundamental flaw to me and perhaps it's time to
>> > reconsider moving to Gerrit.
>> 
>> Stop right there.  This debate has chewed up about 25 hours of
>> developer time so far, with no end in sight.  I realize that
>> you're an excellent person to move it forward, but I don't want to
>> hear about it right now.
>> 
>> I'm vetoing this discussion for 5 weeks.  Wait until you know us
>> better (in particular, the relative lack of technical ability),
>> wait until we know you better, wait until our Grand Organization
>> Project starts up again.  this, incidently, is exactly GOP 13:
>> http://lilypond.org/~graham/gop/gop_13.html
>
>OK, I couldn't remember whether I'd already seen it somewhere :-)
>Just wanted to make sure it's on people's radar, so given GOP 13 I'm
>happy not to mention it again :-)

Rietveld doesn't prevent multiple patches per issue.  Our policy of
testing each commit to ensure that it doesn't break build prevents
multiple patches per issue.
>
>> Moving back to the jazz patches: Carl, could you take a look at
>> his git repo and suggest any way of moving forward?
>
>Thanks, guidance would be welcome.

I will do so, but probably not until Saturday.

Thanks,

Carl