Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

fix for dbic-migration help command + test #1

Merged
merged 3 commits into from Feb 29, 2012

Conversation

Projects
None yet
2 participants
Contributor

felliott commented Feb 29, 2012

Hello,

On my system dbic-migration help doesn't produce any output. I think this is because pod2usage doesn't know which module to pull the pod from. According to the Pod::Usage docs, this can be fixed by passing the -input arg to pod2usage. I've added this and it works on my system. I've also added a test to make sure it produces the correct output. I've split this into 3 commits:

1.) HEAD^^ fixes the bug using Pod::Find per the Pod::usage docs

2.) HEAD^ adds Pod::Find to the prereqs in dist.ini. Pod::Find is in the same dist as Pod::Usage, and I wasn't sure if you liked explicit dependencies or not.

3.) HEAD adds a new test script, t/script.t, that checks the output of the help command. One caveat: since pod2usage calls exit(), I had to add a dependency on Test::Trap in order to capture the output. Test::Trap currently has failing tests on my system, though they're minor (in comparing error messages, a trailing period was missed). I could put it in a SKIP block, if you'd prefer.

Cheers,
Fitz

jjn1056 added a commit that referenced this pull request Feb 29, 2012

Merge pull request #1 from felliott/master
This looks great!  I do have a longer term plan for better command line help, but this is a good start.  Thanks!  I'll send this to cpan a bit later today or tonight

@jjn1056 jjn1056 merged commit 137de5a into jjn1056:master Feb 29, 2012

Owner

jjn1056 commented Feb 29, 2012

BTW, I'll poke around Test::Trap a bit and see if I can find someone to fix this

Contributor

felliott commented Feb 29, 2012

Thank you! I submitted a bug report + patch to RT: https://rt.cpan.org/Public/Bug/Display.html?id=75430

Cheers,
Fitz

Owner

jjn1056 commented Mar 1, 2012

Hey,

just FYI this in on the way to cpan, along with yet another fix for a failing test case (unrelated to you patch). I decided to make the help test option and run only if Test::Trap is already installed. I will consider that an author side test for now, until trap trap stabilizes a bit more. Thanks for the patch and more help me make this dist better and easier to use.

Johnn

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