Re: Macro for(UP_and_DOWN) and 3 similar. (issue 2491) (issue 6109046)

[David Kastrup]

Łukasz Czerwiński <address@hidden> writes:

> On 25 April 2012 11:57, <address@hidden> wrote:
>
>     It would appear that you ignored
>     <http://permalink.gmane.org/gmane.comp.gnu.lilypond.auto/465>,
>     <http://permalink.gmane.org/gmane.comp.gnu.lilypond.auto/466>,
>
>
> No - just look at dates - the comments are newer than my third patch
> set.

Please take a look at
<URL:http://code.google.com/p/lilypond/issues/detail?id=2491> (as the
reporter of this issue, you are copied with every message as well).


The order of code review and resubmission is:

Comment 1 by project member address@hidden, Apr 23 (44 hours ago)

Patchy the autobot says: LGTM.  But bad formatting everywhere: should be "for 
(UP_and_DOWN (d))"

Labels: -Patch-new Patch-review
Comment 2 by project member milimetr88, Yesterday (19 hours ago)

Macro for(UP_and_DOWN) and 3 similar. 
Replaces do{ ... } while(flip (&d) != DOWN/UP/... with a macro 
for(DOWN_and_UP/UP_and_DOWN/....) { }

http://code.google.com/p/lilypond/issues/detail?id=2491

http://codereview.appspot.com/6109046

Labels: -Patch-review Patch-new
Delete comment
Comment 3 by project member address@hidden, Yesterday (17 hours ago)

Patchy the autobot says: Lots of regtest differences.  Example: irregular right 
border for compound-time-signatures.ly (particularly bar 8 sticks out to the 
right), also programming errors "Adding reverse rod" like in ambitus-gap.ly

Labels: -Patch-new Patch-needs_work

Comment 4 by project member milimetr88, Today (15 hours ago)

Macro for(UP_and_DOWN) and 3 similar. 
Replaces do{ ... } while(flip (&d) != DOWN/UP/... with a macro 
for(DOWN_and_UP/UP_and_DOWN/....) { }

Labels: -Patch-needs_work Patch-new


So I do a code review in comment #1 and tell you that the formatting is
wrong, and how.  You resubmit a patch _without_ changing that and set
the status to Patch-new, asking for another review in comment #2.  I
tell you that this patch does not pass the review because of _lots_ of
regtest differences, and tell you examples in comment #3.  Two hours
later, you resubmit a patch in comment #4 that shows the _same_ regtest
differences that I rejected the patch for.

To me, that does not look like you particularly value getting a review.
You have not fixed a single thing I pointed out.  You have not checked
your submission yourself for the problems.

My time (and my computer's time) on this issue has been totally wasted
up to now.

We have a review process for a reason.

>     As I understand, fixxcc.py will correct that automatically, so I
>     don't have to bother about that? Nevertheless I'll remember that
>     for my future patches.

Automated changes like that of fixxcc.py make problems when merging and
rebasing.  So we usually try to catch stuff like that in advance.  There
is no point in creating more problems when one _knows_ about them
already.

>     <http://permalink.gmane.org/gmane.comp.gnu.lilypond.auto/442>.
>     
> I didn't notice that comment. I'm not used yet to checking comments on
> Google Code.

You got the respective messages by personal mail as well unless
something went very wrong.

>     Code reviews take time, effort, and diligence.  Patch testing can
>     be done by yourself easily.  If it isn't, it again takes time,
>     effort, and diligence.
>     
>     It is a matter of courtesy not to waste those lightly, and treat
>     the resources of your coworkers with the respect you want to have
>     them treat yours.
>     
>
> For me it sounds like blaming me that I'm a beginner developer on
> Lilypond project, so my work isn't as optimized as it should be.

No, it is blaming you for ignoring feedback and mails sent to you with
reviews.

> It's not nice for me, really, and it doesn't encourage me to submit my
> patches either.
>
> I'd like to know how to run regtests. Should I
> follow: 
> http://lilypond.org/doc/v2.15/Documentation/contributor/regtest-comparison
>
> and compare all those tests that differ?

Yes, that is the respective instruction.  You get a visual comparison
for the tests that differ significantly.

But even if you don't run "make check" yourself: if I go to the pains of
listing bad files and symptoms in a review, submitting a patch without
checking at least those files means that the time and effort I spent on
the review is worth crap to you.

If your submission was treated like that, you'd be right to complain.
Instead you complain that your patches get tested, evaluated, and
repeated errors reported to you.  This takes work to do, work that
several people have volunteered for, and which nobody except myself has
actually done for several months now.

This is not motivating.  And it does not help if the work gets ignored
and one gets called a bugbear for it, to boot.

-- 
David Kastrup