New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- Added parameter validation code to the method new_page(). #2

Merged
merged 2 commits into from Sep 24, 2015

Conversation

Projects
None yet
2 participants
@manwar
Owner

manwar commented Sep 18, 2015

Hi Gabor,

I am proposing the following changes:

- Added parameter validation code to the method new_page().
- Added parameter validation code to the method font().
- Added new unit test 10-generic.t to reflect the above changes.
- Minor changes to the description of method new_page() in the pod document.
- Minor changes to the description of method font() in the pod document.

Please review it,

Many Thanks.

Best Regards,
Mohammad S Anwar

manwar added some commits Sep 18, 2015

- Added parameter validation code to the method new_page().
- Added parameter validation code to the method font().
- Added new unit test 10-generic.t to reflect the above changes.
- Minor changes to the description of method new_page() in the pod document.
- Minor changes to the description of method font() in the pod document.
Renamed the @valid_keys appropriately as suggested by Gabor.
Also removed -w from the first line in the test script and used 'use warnings' instead.
@manwar

This comment has been minimized.

Show comment
Hide comment
@manwar

manwar Sep 24, 2015

Owner

Hi Gabor,

Thanks for reviewing my changes.
I agree with you with respect to the naming the variables appropriately. I have changed it accordingly. Regarding unit test using "-w", I just wanted to follow the same style as other test scripts. I didn't want to impose my own style. But as you correctly pointed out BEGIN {} statement is unnecessary. I have updated the test script as well.

Please review the changes when you get time.

Many Thanks.

Best Regards,
Mohammad S Anwar

Owner

manwar commented Sep 24, 2015

Hi Gabor,

Thanks for reviewing my changes.
I agree with you with respect to the naming the variables appropriately. I have changed it accordingly. Regarding unit test using "-w", I just wanted to follow the same style as other test scripts. I didn't want to impose my own style. But as you correctly pointed out BEGIN {} statement is unnecessary. I have updated the test script as well.

Please review the changes when you get time.

Many Thanks.

Best Regards,
Mohammad S Anwar

szabgab added a commit that referenced this pull request Sep 24, 2015

Merge pull request #2 from Manwar/add-unit-test-parameter-validation
- Added parameter validation code to the method new_page().

@szabgab szabgab merged commit a932ce0 into manwar:master Sep 24, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@szabgab

This comment has been minimized.

Show comment
Hide comment
@szabgab

szabgab Sep 24, 2015

Collaborator

Thanks.
Then you might want to fix all the other tests as well :)
You can also run Devel::Cover to see what else is missing tests.

Collaborator

szabgab commented Sep 24, 2015

Thanks.
Then you might want to fix all the other tests as well :)
You can also run Devel::Cover to see what else is missing tests.

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