Some additions #1

Closed
wants to merge 9 commits into
from

Conversation

Projects
None yet
2 participants

trapd00r commented Oct 3, 2011

Hi Ivan,

See the commit messages for descriptions.

trapd00r added some commits Oct 3, 2011

allow for other countries to be specified
Replaced conditional and use with require and ""->can
remove the usage of warnings
It's a module - we should either fail as soon as possible as loud as possible or
be quiet.
samle.pl: rewrite
Show how to use different target countries.
Use lib.pm so the script can be run without installing the whole package.
Owner

bessarabov commented Oct 3, 2011

Thank you for the great work!

I agree with you that module should die as soon as possible, but I don't want to remove warnings. I'm thinking about replacing warnings with dying (as described here http://www.effectiveperlprogramming.com/blog/310).

I'm not sure that Mock::Person::SV should be part of this module. You have already published in on CPAN, so I think It is better be published under your name so you will maintain it in your own repo https://github.com/trapd00r/Mock-Person-SV

So I'm planning to do 2 things with your code: 1) remove Mock::Person::SV 2) die on any warnings. What do you think?

trapd00r commented Oct 3, 2011

Yeah, I wasn't sure if you wanted to distribute other languages with it; I do think, however, that it'd be better to have an english wordlist as the default one. Or alternatively, not include any wordlist at all in Mock::Person, and have subclasses for each language. It's a bit weird having the russian one included with the dist and nothing more. :)

But I'm fine with whatever you prefer.

Owner

bessarabov commented Dec 13, 2013

You did a great work for the Mock::Person module. Sorry for not merging this PR. And sorry, I was mistaken that we should split this into several CPAN distributions. I'm closing this PR without merge, because I'm doing the big change of the module and the things that you propose here are done in the git branch big_change. The discussion of this change is at #7

@bessarabov bessarabov closed this Dec 13, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment