[frogs] Re: T1247 - Conditionally do (use-modules (ice-9 curried-definitions)) if running with Guile V2, (issue2219044)

[ Thread Index | Date Index | More lilynet.net/frogs Archives ]


I like the code for choosing Guile V2.

I'm concerned about a bunch of the re-indentation changes.

Thanks,

Carl



http://codereview.appspot.com/2219044/diff/1/scm/lily.scm
File scm/lily.scm (right):

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode197
scm/lily.scm:197: ;(set-debug-cell-accesses! 1000)
Why is this shoved over to the right?

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode199
scm/lily.scm:199: ;;; Boolean thunk - are we integrating Guile V2.0 or
higher with Lilypond?
It should either be LilyPond (the application) or lilypond (the
executable).

I don't think it should ever be Lilypond.

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode204
scm/lily.scm:204: (ice-9 safe)
These should all be aligned with (ice-9 regex) as in the old code.

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode300
scm/lily.scm:300: (vector-ref (uname) 0) char-set:letter+digit)))
(vector-ref should be indented; it's an argument to string-tokenize.

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode337
scm/lily.scm:337: iface))
Looks like there are trailing spaces here?

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode568
scm/lily.scm:568: r5rs-secondary-predicates
Why the move from spaces to tabs?  As far as I know, the tabs-vs-spaces
discussion is not settled.  Given that it's not, I don't think we should
be changing existing code when that's the only change.

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode829
scm/lily.scm:829: (ly:stderr-redirect (format "~a.log" base) "w"))
The previous indentation here was correct.  Then clause should be
aligned with if clause

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode831
scm/lily.scm:831: (format ping-log "Processing ~a\n" base))
Same here

http://codereview.appspot.com/2219044/diff/1/scm/lily.scm#newcode848
scm/lily.scm:848: (ly:reset-all-fonts))))
Else clause should be aligned with if clause.

http://codereview.appspot.com/2219044/

---
----
Join the Frogs!


Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/