http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc File lily/accidental-placement.cc (right):
http://codereview.appspot.com/5651069/diff/1/lily/accidental-placement.cc#newcode211 lily/accidental-placement.cc:211: * @return A vector of Accidental_placement_entrys Do you mean @param and @return or the comment to the function? What comment would you propose? On 2012/02/11 00:09:00, joeneeman wrote:
Please don't just comment on the type (unless it's SCM in which case
you can say
which scheme type it is).
http://codereview.appspot.com/5651069/diff/1/lily/include/note-column.hh File lily/include/note-column.hh (right): http://codereview.appspot.com/5651069/diff/1/lily/include/note-column.hh#newcode39 lily/include/note-column.hh:39: /** Ok, I'll correct that. On 2012/02/10 23:49:24, Carl wrote:
IMO, this comment belongs in the definition, not the declaration
http://codereview.appspot.com/5651069/diff/1/lily/include/note-column.hh#newcode39 lily/include/note-column.hh:39: /** On 2012/02/10 23:49:24, Carl wrote:
IMO, this comment belongs in the definition, not the declaration
Done. http://codereview.appspot.com/5651069/diff/1/lily/include/staff-symbol-referencer.hh File lily/include/staff-symbol-referencer.hh (right): http://codereview.appspot.com/5651069/diff/1/lily/include/staff-symbol-referencer.hh#newcode53 lily/include/staff-symbol-referencer.hh:53: /** Ok, I'll correct that. On 2012/02/10 23:49:24, Carl wrote:
Again, comment in the definition, not the declaration
http://codereview.appspot.com/5651069/diff/1/lily/include/staff-symbol-referencer.hh#newcode53 lily/include/staff-symbol-referencer.hh:53: /** On 2012/02/10 23:49:24, Carl wrote:
Again, comment in the definition, not the declaration
Done. http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc File lily/note-collision.cc (right): http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc#newcode377 lily/note-collision.cc:377: /** AFAIK, not really. In Contributor's Guide there are only 2 short paragraphs: Comments may not be needed if descriptive variable names are used in the code and the logic is straightforward. However, if the logic is difficult to follow, and particularly if non-obvious code has been included to resolve a bug, a comment describing the logic and/or the need for the non-obvious code should be included. There are instances where the current code could be commented better. If significant time is required to understand the code as part of preparing a patch, it would be wise to add comments reflecting your understanding to make future work easier. (http://lilypond.org/doc/v2.15/Documentation/contributor-big-page#code-comments) I just wanted to introduce JavaDoc comment style for autodocumenting. Maybe style of comments should be discussed on lilypond-devel? On 2012/02/10 23:49:24, Carl wrote:
I don't like the double * following the /. Is this a standard
anywhere else in
lilypond?
http://codereview.appspot.com/5651069/diff/1/lily/note-collision.cc#newcode552 lily/note-collision.cc:552: for_UP_and_DOWN (d) // please, make a comment to this loop (better than the above one...) On 2012/02/10 23:49:24, Carl wrote:
To whom is the "please" directed? Is there anybody who is better than
you right
now to comment the loop?
Yes - someone who knows this code :] Possibly one day someone will try to refactor this code, what will mean that he understands it well. For me it's really hard to understand whats going on here. Me and Janek recently spent a couple of hours trying to understand several files including this one and we still don't know if clash_groups[d] is one single note or a group of notes. Do you have a tip, how to work it out? Unfortunately it's not state in the code. http://codereview.appspot.com/5651069/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel