Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(279)

Issue 4917044: lily-guile updates and CG: "Scheme->C interface" section.

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 10 months ago by Bertrand Bordage
Modified:
7 years, 8 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

lily-guile updates and CG: "Scheme->C interface" section.

Patch Set 1 #

Total comments: 19

Patch Set 2 : Applies Carl and Neil's comments; rewrite the doc. #

Patch Set 3 : Typo. #

Patch Set 4 : Typo again. #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats (+129 lines, -16 lines) Patch
M Documentation/contributor/programming-work.itexi View 1 2 3 3 chunks +113 lines, -1 line 6 comments Download
M lily/general-scheme.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/include/lily-guile.hh View 1 2 chunks +6 lines, -6 lines 0 comments Download
M lily/lily-guile.cc View 1 3 chunks +9 lines, -8 lines 3 comments Download

Messages

Total messages: 19
Bertrand Bordage
Hi, Graham asked me to document an small issue. (http://codereview.appspot.com/4875054/) And this became something bigger. ...
7 years, 10 months ago (2011-08-19 17:05:37 UTC) #1
Graham Percival (old account)
Doc part LGTM. I can't speak about the scm / C++ stuff.
7 years, 10 months ago (2011-08-19 17:10:03 UTC) #2
Carl
THanks for doing this! I have some comments about the docs. I think they're too ...
7 years, 10 months ago (2011-08-19 18:04:38 UTC) #3
Neil Puttock
http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc File lily/general-scheme.cc (right): http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc#newcode110 lily/general-scheme.cc:110: if (scm_is_integer (s)) On 2011/08/19 18:04:38, Carl wrote: > ...
7 years, 10 months ago (2011-08-19 20:20:12 UTC) #4
Neil Puttock
On 2011/08/19 20:20:12, Neil Puttock wrote: > http://codereview.appspot.com/4917044/diff/1/lily/include/lily-guile.hh > File lily/include/lily-guile.hh (right): > > http://codereview.appspot.com/4917044/diff/1/lily/include/lily-guile.hh#newcode96 ...
7 years, 10 months ago (2011-08-19 20:22:08 UTC) #5
Bertrand Bordage
On 2011/08/19 18:04:38, Carl wrote: > THanks for doing this! And thanks for this excellent ...
7 years, 10 months ago (2011-08-19 20:22:27 UTC) #6
Carl
http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc File lily/general-scheme.cc (right): http://codereview.appspot.com/4917044/diff/1/lily/general-scheme.cc#newcode110 lily/general-scheme.cc:110: if (scm_is_integer (s)) On 2011/08/19 20:20:12, Neil Puttock wrote: ...
7 years, 10 months ago (2011-08-19 21:08:10 UTC) #7
Graham Percival
On Fri, Aug 19, 2011 at 06:04:38PM +0000, Carl.D.Sorensen@gmail.com wrote: > I have some comments ...
7 years, 10 months ago (2011-08-19 22:35:35 UTC) #8
Neil Puttock
On 2011/08/19 21:08:10, Carl wrote: > As an aside, I think that we should change ...
7 years, 10 months ago (2011-08-19 23:03:37 UTC) #9
c_sorensen
On 8/19/11 4:35 PM, "Graham Percival" <graham@percival-music.ca> wrote: > On Fri, Aug 19, 2011 at ...
7 years, 10 months ago (2011-08-19 23:07:14 UTC) #10
Carl
On 2011/08/19 23:03:37, Neil Puttock wrote: > On 2011/08/19 21:08:10, Carl wrote: > > > ...
7 years, 10 months ago (2011-08-20 06:24:45 UTC) #11
Bertrand Bordage
Update done. I think the C part is ok. There's maybe a few things to ...
7 years, 9 months ago (2011-09-22 11:41:39 UTC) #12
pkx166h
Couldn't find a tracker issue for this, however it passes make and reg tests.
7 years, 9 months ago (2011-09-22 19:24:19 UTC) #13
Graham Percival (old account)
Since it has C changes as well, I'd prefer it to go through a countdown.
7 years, 9 months ago (2011-09-22 20:53:39 UTC) #14
Bertrand Bordage
I pushed the doc as b4a2cb2cf00347c477ed595f1435cc212e70ce33. Could the remaining C part of the patch be ...
7 years, 8 months ago (2011-10-18 13:10:43 UTC) #15
Bertrand Bordage
"lily-guile updates" pushed as 6ee8c04678442855cb794d4598c056c15c42673b.
7 years, 8 months ago (2011-10-23 20:58:15 UTC) #16
dak
As usual, too late in the game. Better late than never. http://codereview.appspot.com/4917044/diff/16006/Documentation/contributor/programming-work.itexi File Documentation/contributor/programming-work.itexi (right): ...
7 years, 8 months ago (2011-10-23 21:23:11 UTC) #17
Bertrand Bordage
Hi David, Could you make your own patch for the doc changes? And, as you ...
7 years, 8 months ago (2011-10-25 17:00:37 UTC) #18
dak
7 years, 8 months ago (2011-10-25 20:51:48 UTC) #19
bordage.bertrand@gmail.com writes:

> Hi David,
>
> Could you make your own patch for the doc changes?

Pushed.  I have enough pending reviews, commits and patches that I don't
have the nerve to do yet another hoop-jumping exercise in parallel.

> And, as you mentionned, the unused function should be removed.  Do you
> want me to commit this change?

That would be the smart thing to do.  Broken unused code does not help
anybody.

-- 
David Kastrup
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b