LGTM, with a few comments. Thanks,
Carl http://codereview.appspot.com/4854049/diff/2001/lily/skyline.cc File lily/skyline.cc (right): http://codereview.appspot.com/4854049/diff/2001/lily/skyline.cc#newcode417 lily/skyline.cc:417: buildings_ = internal_build_skyline (&boxes, horizon_padding, a, UP); What if instead of changing X_AXIS to a in line 417, we checked a to see if it were equal to X_AXIS and issued a warning (or maybe a programming error)? Note that we have UP hardcoded as well, so at this point I don't think we ought to remove the hardcoding on only the a. http://codereview.appspot.com/4854049/diff/2001/ly/music-functions-init.ly File ly/music-functions-init.ly (right): http://codereview.appspot.com/4854049/diff/2001/ly/music-functions-init.ly#newcode894 ly/music-functions-init.ly:894: (_i "Scale @var{arg} up by a factor of 2^@var{dur}*(2-(1/2)^@var{dots}).") I think the whole expression 2^... should be wrapped in a @code{} block. Also, should this be part of the warning fix patch? http://codereview.appspot.com/4854049/diff/5001/lily/lyric-combine-music-iterator.cc File lily/lyric-combine-music-iterator.cc (right): http://codereview.appspot.com/4854049/diff/5001/lily/lyric-combine-music-iterator.cc#newcode224 lily/lyric-combine-music-iterator.cc:224: Lyric_combine_music_iterator::check_new_context (SCM /*sev*/) Why not /*SCM sev*/ with a FIXME? http://codereview.appspot.com/4854049/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
