Re: Various clean-ups in stems and beams. (issue 6584045)

[dak]

On 2012/11/03 11:37:08, mike7 wrote:
> On 3 nov. 2012, at 12:26, mailto:address@hidden wrote:

> >
> > http://codereview.appspot.com/6584045/diff/1/lily/beam.cc
> > File lily/beam.cc (right):
> >
> > http://codereview.appspot.com/6584045/diff/1/lily/beam.cc#newcode197
> > lily/beam.cc:197: Grob *me = unsmob_grob (smob);
> > Looking at the combination of this and is_kievan, it would appear
that
> > the expected response when calling Beam::calc-is-kievan (why no
question
> > mark in the name?) with a non-Grob is a segmentation fault.
> >
> > That's sub-fabulous.
> >

> Quick response - if you're looking at changes in beam.cc, you are
reviewing an
> old patch set from a month ago or so.

The problem with incremental changes like this is that the humongous
total change set is hard to review, and the incremental change is
considered "outdated".

If we were reviewing this as a commit series of git, every commit in the
series of only loosely related changes would be expected to be sane.
With Rietveld, one can't really distinguish between history of changes
and sequence of commits.

So I apologize for having looked at this in parts, but it would appear
that the LY_ASSERT_TYPE thing applies equally well to the latest change.

http://codereview.appspot.com/6584045/