Mike,
again i was outrageously slow, i apologize.
Thanks for the explanations, i see now that the code is pretty clear
when you're accustomed to our code base a little.
I think the docstring you wrote is everything that's needed for this, so
no need to do anything more here.
Sorry for delay. I
Reviewers: Patrick McCarty, dak, mike_apollinemike.com, janek,
http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc
File lily/grob.cc (right):
http://codereview.appspot.com/5504106/diff/11013/lily/grob.cc#newcode177
lily/grob.cc:177: SCM expr = scm_list_3 (ly_symbol2scm ("id"),
On 2012
Hi Mike,
i apologize for the delay; i focused on other things that seemed more
urgent to me.
On 2012/01/11 12:27:10, mike_apollinemike.com wrote:
[explanation of the patch]
I'm not sure how/where to include this info in the source: if you can
think of a
good way to phrase it that would mak
On Jan 11, 2012, at 12:53 PM, janek.lilyp...@gmail.com wrote:
> Hi Mike,
>
> could you add some comments to the code and/or commit message explaining
> what it does? I've read whole patch and i don't understand what happens
> here, except that it's some kind of XML identifier.
>
> tia,
> Janek
Hi Mike,
could you add some comments to the code and/or commit message explaining
what it does? I've read whole patch and i don't understand what happens
here, except that it's some kind of XML identifier.
tia,
Janek
http://codereview.appspot.com/5504106/
_
"m...@apollinemike.com" writes:
> I also fixed this in the reverted patch. Unfortunately, I did not
> save the final pushed version of the patch - do you have a copy of it?
> If so, I won't need to make the changes again.
Unless you scrapped your whole repository,
git cherry-pick 6abea44be29f2
On Jan 6, 2012, at 2:35 PM, d...@gnu.org wrote:
> Is there a reason you have ignored Patrick's comments?
>
They were incorporated into the version that was pushed to staging.
> The issue is missing a description, so is any part of the code.
>
> It needs at least a regtest demonstrating the use
Is there a reason you have ignored Patrick's comments?
The issue is missing a description, so is any part of the code.
It needs at least a regtest demonstrating the use of this feature,
otherwise nobody will be able to ever put this code to actual use (or
write user documentation for it) if you
LGTM
https://codereview.appspot.com/5504106/diff/1/scm/output-ps.scm
File scm/output-ps.scm (right):
https://codereview.appspot.com/5504106/diff/1/scm/output-ps.scm#newcode258
scm/output-ps.scm:258: (define (open-node n) n)
I don't see this procedure used anywhere...
https://codereview.appspot
