robkinyon / dbm-deep

DBM::Deep Perl module
http://search.cpan.org/dist/DBM-Deep/
11 stars 16 forks source link

Add missing strictures #13

Closed shlomif closed 9 years ago

shlomif commented 9 years ago

CPANTS Kwalitee.

shlomif commented 9 years ago

Thanks!

ribasushi commented 9 years ago

@shlomif FATAL => 'all' prliferation is a generally bad idea and is officially discouraged by p5p. Please consider following up with a reversal PR.

shlomif commented 9 years ago

@ribasushi : hello! The «FATAL => 'all'» for the «use warnings;» lines exist in all other .pm source files in DBM-Deep, so I added it in the missing .pm as well for consistency. I realise it's a bad idea and avoid using it for my own code. Maybe you should file an issue for it for DBM-Deep.

robkinyon commented 9 years ago

@ribasushi - The official discouragement came years after I stopped actively working on DBM::Deep. I'd be happy to accept a PR that fixes the problem correctly (instead of just removing the FATAL => 'all').

ribasushi commented 9 years ago

@robkinyon my comment wasn't a critique of the module as a whole, just of this PR. Blindly removing FATAL's without a code audit is just as dangerous as adding them where there were none (what this PR did). To put it in terms an engineer would understand:

In any case - the original comment was simply made in passing, about something minor-ish I noticed. No action being taken is a perfectly reasonable outcome.