Nothing obviously wrong in a regtest comparison, and the code style looks fine to me.
http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc File lily/part-combine-iterator.cc (right): http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode69 lily/part-combine-iterator.cc:69: Stream_event *unisono_event_; On 2011/01/25 17:11:19, Reinhold wrote:
On 2010/11/26 23:05:51, Valentin Villenave wrote: > How about declaring these variables on the same line?
I don't think that's our coding style.
I agree; let's keep the variable declarations separate. They can be grouped by adding extra newlines if necessary (as they are already). http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode110 lily/part-combine-iterator.cc:110: void broadcast_new_state (Music_iterator *new_active, Stream_event *new_state_event); On 2011/01/25 17:11:19, Reinhold wrote:
On 2010/11/26 23:05:51, Valentin Villenave wrote: > I'm not familiar with C++ coding style, but that looks like an
awfully long
> line.
I can shorten the argument names, that would save a few characters...
I think it's clearer to read them as is. I wouldn't call that a really long line. 87 chars is getting a bit uncomfortable, but it's not terrible. Other places in our code go over by more. If any change were to be made here, I'd suggest a newline before "Stream_event". http://codereview.appspot.com/3334043/diff/1/lily/part-combine-iterator.cc#newcode110 lily/part-combine-iterator.cc:110: void broadcast_new_state (Music_iterator *new_active, Stream_event *new_state_event); On 2011/01/25 17:11:19, Reinhold wrote:
On 2010/11/26 23:05:51, Valentin Villenave wrote: > I'm not familiar with C++ coding style, but that looks like an
awfully long
> line.
I can shorten the argument names, that would save a few characters...
I think it's clearer to read them as is. I wouldn't call that a really long line. 87 chars is getting a bit uncomfortable, but it's not terrible. Other places in our code go over by more. If any change were to be made here, I'd suggest a newline before "Stream_event". http://codereview.appspot.com/3334043/ _______________________________________________ lilypond-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/lilypond-devel
