On Sun Jan 09 20:39:58 2011, WYANT wrote:
Show quoted text> I'm still not completely sure where I come out on this. My knee-jerk is
> that we should accept the patch. But when I think about it, I tend to
> believe we should not, at least as submitted. I hate that, especially
> when someone has gone to the trouble to track down the behavior they
> would like to change, and come up with a patch for it. It seems a poor
> reward for their diligence.
>
In my case, I am rewarded sufficiently by your thoughtful responses, so
no worries!
Show quoted text> When I think about it, my model is deleting a file. It can be argued
> that it should not be an error when you try to delete a file which is
> not there, because the end state is the same -- no file. But whether you
> spell delete 'rm', 'del', 'delete', or even 'pip /de', every
> command-line deletion program I can think of warns on an attempt to
> delete a non-existent file, at least by default. Most of them also have
> some way to suppress warnings, and the suppression is not limited to the
> warning about a non-existent file.
>
> If that analogy is any good, then Perl::Critic already works that way.
> But in that case I'm inclined to see the request as a wish for a way to
> suppress warnings on disabling non-existent policies, without
> suppressing anything else. And the question becomes how best to do that.
>
> ...
> Of course, no matter what we do, the requester has to upgrade
> Perl::Critic to get the benefit. But an upgrade would fix his problem
> even if we did nothing.
>
> Make of all this what you will.
>
> Tom
I'm more and more persuaded that just suppressing the warning for the
case I don't like is not the right thing. It makes the 'strictness'
parameter too "potentially surprising". I was definitely surprised by
the behavior I saw, but there is adequate provision in the
-profile-strictness parameter for most usage.
My surprise was really the result of a versioning issue, not a
strictness issue, so I think after all that's where it should be
addressed (if at all). My humble proposal is this:
Add a parameter named '-validate-at-version' which accepts a
Perl::Critic version number, or a range between two versions. The
actions would be as follows:
1. If the Perl::Critic version in use is equal to the highest version in
the validation range, or no parameter is given, validate everything
according to the strictness parameter.
2. If a range is given, and the Perl::Critic version falls within this
range, but is not the highest, then skip validation errors/warnings that
are likely caused by versioning (such as my case where a new Policy is
desired to be disabled). Yes, errors as well, for consistency.
3. If the Perl::Critic version is outside the given range, then add a
warning/error for ProfileVersionMismatch.
Omitting this parameter then behaves exactly as now.
Using a single value enforces a specific version of Perl::Critic for the
given policy.
Using a range value implies that the policy is designed for the latest
Perl::Critic version given, but previous versions within the range are
known to be suitable as well. Probably should warn in the documentation
against using this casually!
While I think this is the right answer, I'm not prepared to supply a new
patch immediately. Hopefully I can find an opportunity in the next few
months if you agree it is worthwhile.
Thanks!
DLS
