Re: [Fwd: Tracker 836:]

[Neil Puttock]

2009/10/28 Ian Hulin <address@hidden>:
> I managed to get set up and put the patch for Tracker 836 on as tracker
> 141076.

This issue doesn't exist; did you delete it after uploading?

> Message:
> ly/engraver-init.ly in not part of this patch and I didn't change it.

You should apply your patch to master, then you won't get previous
commits showing up.

The way I normally do it is as follows:

-) create a patch which you want to upload
-) reset master branch (if necessary, pull any changes)
-) create a new branch for uploading to Rietveld
-) checkout new branch and apply patch
-) upload using `git cl upload origin'

> http://codereview.appspot.com/141076/diff/2001/2006#newcode144
> Line 144: (ly:parser-lookup parser 'book-filename))))
> Coding
> (book-filename))))
> here using the local variable caused an "invalid type" error from Scheme

Please read my suggestion again (and Carl's explanation).

> http://codereview.appspot.com/141076/diff/2001/2006#newcode154
> Line 154: (define-public current-outfile-name #f)  ; for use by
> regression tests
> current-outfile-name was added to help write a regression test for this
> patch.

Don't keep us in suspense then; add the regression test to the patch
so we can review it. :)

There's one more bug with output-count.  I suspect this line

(assoc-set! counter-alist output-suffix (1+ output-count)))

needs tweaking:

(assoc-set! counter-alist alist-key (1+ output-count)))

Cheers,
Neil