Why no 'use warnings' in PPI modules?

[moregan]

The test suite is careful to 'use warnings', but none of the PPI modules that get installed enable warnings. Any insight into the reasoning and/or experience that lead to not turning on warings, @adamkennedy ?

[adamkennedy]

It's been my experience, accumulated over many many years, that on balance enabling warnings in production can sometimes do more harm than good.

For a deep library like PPI (which has no direct use case, and is used as part of some larger tool, possibly with multiple other layers of libraries in the middle) a warning emitted to a user likely has no relevance to what they are doing at all, and there's probably nothing they can do to fix it.

Conversely, if you imagine a situation in which a warning is emitted by the lexer or the tokenizer, there is every chance that warning will be thrown hundreds, thousands or even tens of thousands of times. This not only drowns out any other real messages, but I have in the past experienced some cases in which a warning in a hottish loop completely filled up the disk of a web server with repeated warning messages.

For a large, evolving and very complex codebase like PPI, this is a real possibility.

So for this project, I decided that is was better to disable warnings in production, but force them on and treat every warning as an error in the test suite. The idea being that with a fairly large and quite intensive test suite like PPIs, if there's some problem that it DOESN'T catch, then there's probably nothing you can do about it in production and to impose it on everyone isn't worth the risk of pain it might cause.

[wchristian]

After #206 brought this fact to the fore again, i have decided that adding warnings to all modules, and eventually fatalizing them, is necessary. As stated, PPI is used within other libraries. Many of these libraries might be doing unexpected things, like flipping $^W or even fatalizing warnings.

We cannot write PPI with the expectation that warnings will be disabled inside it.

Another problem was that EUMM used to run tests with -w but was changed to stop doing that. So in order to provide a first level of safety i was forced to add code to the pragmas module that fiddles with the internals to flip the tests on. Of course this is a ticking time bomb in case tests are written that do not use the pragmas.

And another reason for doing this this way was that the coverage of the source code was quite low. With the recently released PPI versions coverage is now at 91% of lines executed, making it considerably less risky to add warnings everywhere.

Further, thanks to @wolfsage we now have a tool to easily test PPI against its downstream dependencies.

And lastly, thanks to having fixed up many things i find myself in a position where i can cut PPI releases a lot more quickly with a lot more confidence, and @karenetheridge is working on changes that will make even the release process itself a lot safer.

And as the cherry on top, with strictures.pm v2 we now have a safe tool to fatalize all warnings, making the nightmare scenario of runaway logs an impossibility.

My plan is thus as follows:

1. Make $^W++ release and let it bake for a while.
2. Make a patch with warnings in all modules, check against downstream deps, release, bake.
3. Make a patch with use strictures 2; in all modules, check against downstream deps, release, bake.