So I went through a review after all and found a few niggles that seem worth fixing. Probably does not require a full countdown and stuff, but another patchy run might be warranted in order to avoid typos.
https://codereview.appspot.com/137760043/diff/20001/lily/grob.cc File lily/grob.cc (right): https://codereview.appspot.com/137760043/diff/20001/lily/grob.cc#newcode470 lily/grob.cc:470: SCM min_ext = get_property (min_ext_name); This one is bad since it breaks memoization. Either leave it alone or do (properly formatted) SCM min_ext = a == X_AXIS ? get_property ("minimum-X-extent") : get_property ("minimum-Y-extent") https://codereview.appspot.com/137760043/diff/20001/lily/self-alignment-interface.cc File lily/self-alignment-interface.cc (right): https://codereview.appspot.com/137760043/diff/20001/lily/self-alignment-interface.cc#newcode54 lily/self-alignment-interface.cc:54: SCM align (me->get_property (sym)); See above. https://codereview.appspot.com/137760043/diff/20001/lily/self-alignment-interface.cc#newcode125 lily/self-alignment-interface.cc:125: ? me->get_property ("self-alignment-X") Here you do it properly! https://codereview.appspot.com/137760043/diff/20001/lily/span-bar-engraver.cc File lily/span-bar-engraver.cc (right): https://codereview.appspot.com/137760043/diff/20001/lily/span-bar-engraver.cc#newcode85 lily/span-bar-engraver.cc:85: SCM vis = bars_[0]->get_property ("break-visibility"); This causes the symbol to be memoized three times in a row. However, only the first time the code is being run, so I'd say never mind. The readability should be worth it. https://codereview.appspot.com/137760043/ _______________________________________________ lilypond-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/lilypond-devel
