On Sep 28, 2011, at 12:35 PM, Reinhold Kainhofer wrote:
> Am Wednesday, 28. September 2011, 11:54:38 schrieb [email protected]:
>> On Sep 28, 2011, at 11:45 AM, Reinhold Kainhofer wrote:
>>> Am Wednesday, 28. September 2011, 09:07:12 schrieben Sie:
>>>> LGTM
>>>>
>>>> It'd be great to see the regtests run with -dwarning-as-error now that
>>>> you've done all this work. Is that possible to incorporate into this
>>>> patch?
>>>
>>> Actually, I don't think that's a good idea, for several reasons:
>>>
>>> 1) The warning-as-error is currently only implemented in the Input class,
>>> not for warnings that are triggered by directly calling (ly:warning...).
>>> So we won't catch all warnings anyway.
>>
>> Is there a way to make it such that they are capture?.
>
> I'm working on it.
>
Sehr gut.
>>> 2) Setting -dwarning-as-error means that any warning (e.g. a failed
>>> barcheck...) will cause a complete failure of the whole regtest build. I
>>> think this is way too volatile to be useful. A build should never fail,
>>> except for real problems.
>>
>> I agree, but perhaps warnings like bar, bar number, and octave checks can
>> also have a throw/catch mechanism like the one you're implementing here.
>
> Sure, if the warning is supposed to be there, it will be suppressed and the
> file won't fail. But I'm talking about those little oversights that happen
> when doing last-second changes before committing.
>
> I don't think that an inadvertent warning should hold up all other developers
> from testing their own patches.
>
I feel that the category of "little oversights that happen when doing
last-second changes before committing" applies to other mistakes (forgotten
semi-colons, syntax errors, etc.). All of these will crash build, all of these
are inadvertent, and all of these will stop development. In general, the
argument "Law X, which rights wrong Y, will turn Y into criminality, which will
be an inconvenient for the taxpayers" seems to be OK in the short term but
potentially problematic in the long term :)
>> True, but the whole reason these problems show up in the master branch in
>> the first place is because people (including me) accidentally push stuff
>> that generates warnings. If the regtest build failed because of
>> -dwarning-as-error, then the warning-generating code would not be pushed
>> (assuming the person is not a malevolent pusher and/or lazy). That's the
>> big advantage I see of using -dwarning-as-error for regtests.
>
> Well, I forgot my last reason:
>
> 3) We currently have several warnings in the regtest that should not be
> there.
> To see this, simply run "lilypond -l BASIC *.ly" in input/regression.
>
I wouldn't mind scrubbing these.
> With this patch applied, the only displayed warnings should be those from
> safe.ly and warn-expected-warning-missing.ly.
>
> However, there are many more. Those are not expected, so suppressing them is
> like painting over the rust without actually fixing it. Unless all those are
> fixed, we simply cannot enable warning-as-error.
>
I see what you mean. A couple things:
1) If a regtest exists to show painted rust (several do), this is not a
problem.
2) You're right that this'll force us to get on bugs that we've been negligent
about, but at the same time, to merit the status of bug-free that we claim the
regtests portend, this should be the goal. Granted, I think that these things
should be fixed before applying -dwarning-as-error, but it seems like a good
target.
3) There could be a type of warning suppression called "open-question" (or
whatever) where we're not sure why the warning exists, or if there's anyway to
overcome it, or whatever. This can be wrapped about these warnings and written
to the log file or console. This way, we'll have a record of the rust we've
painted over. The only danger then is, of course, if someone as an expedient
hides a warning that she shouldn't hide, but I think this behavior will be
generally frowned upon and not engaged in.
Cheers,
MS
_______________________________________________
lilypond-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/lilypond-devel
