[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Corrected style of comments (issue 5862052)

From: milimetr88
Subject: Re: Corrected style of comments (issue 5862052)
Date: Sat, 31 Mar 2012 16:13:12 +0000

I'm sorry for that, but the next bunch of corrections are in a separate

It seems that following the procedure from
does not make git-cl ask for the issue number. Instead it creates a new
one. How to change that? Always call 'git cl issue' before 'git cl
File flower/include/direction.hh (right):
flower/include/direction.hh:77: * Thanks to a #define below, instead of
On 2012/03/23 07:23:36, Keith wrote:
> Only you and Han Wenn have answered. Maybe other agree on
> changing do to for_UP_and_DOWN? How do I know?

You cannot know every individual opinion, so you must decide what is
wise.   You
know that I fear that I will forget if there is any difference between
do{}while(flip()) and for_UP_and_DOWN.  You know that HanWenn suggests
all do{}while(flip) at once.

If you change just a few do{}while(flip) to your new idiom, then I
will be
tempted to create a third idiom:
  for (Direction d = UP; d >= DOWN; d = (Direction)(d + DOWN - UP)

Yes, you are right - I should have changed all occurences at once. There
are 136 such. Do you have an idea, if it could be done automatically?
File lily/ (right):
lily/ //TODO: add comment to this class
On 2012/03/22 14:55:38, Graham Percival wrote:
I'm not certain this line adds anything.  Also, isn't it a struct, not
a class?

Well, I think it adds something - a request to comment a struct that is
not obvious one.
Yes, it's a struct, not a class. AFAIR it's not a big difference in C++,
but ok, I'll correct the comment.
File lily/ (right):
lily/ /* Filter out the 'o's in this configuration,
since they're no
Keith, I'm trying to understand the whole function and divide it into
Does this comment refer only to the two lines below?
lily/ */
On 2012/03/23 07:23:36, Keith wrote:
Move the block comment above down below your new comments, to keep it
to the if...elseif... block that it describes.

lily/ // The offset should depend on line
thickness, not staff space, at least in some cases (like stem-to-stem,
where it should be bigger for smaller font size)
On 2012/03/23 07:23:36, Keith wrote:
What does "the offset" refer to?  shift_amount?

Yes. Corrected.
lily/ void update_offsets (Drul_array<vector<Real>
Well, ok, the function is indeed too short. But the whole function
check_meshing_chords should be split, don't you think? Currently it has
over 300 lines!

reply via email to

[Prev in Thread] Current Thread [Next in Thread]