Skip to content
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

Made all tests roughly the same style #11

Merged
merged 1 commit into from
Feb 18, 2019

Conversation

genio
Copy link
Contributor

@genio genio commented May 21, 2016

Hi! I was assigned your module in this month's PR challenge. While reviewing the module, nothing immediately jumped out at me that needed a fix. So, I had to go nitpicking (no- really, really nitpick-y). The test files were all slightly different in their beginnings, so I looked through them and found what seemed to be the most common theme and updated them all accordingly.

Some test files had a shebang line, some didn't. Some had strict and warnings, others didn't. They all now mostly start with a consistent first few lines:

#!perl

use strict;
use warnings;

Two not quite as nitpick-y as the first changes were also made:

  • In 00-load.t it seems beneficial to go ahead and BAIL_OUT if the use_ok fails
  • And, in test.t and text_vs_data.t, use $x and $y rather than $a and $b as they are special.

Thanks,
Chase

…arnings;). No real changes to tests. Also, use $x and $y rather than $a and $b as they are special. Bail out if we cannot use Test::Differences
@DrHyde DrHyde changed the base branch from master to genio-test-style February 18, 2019 23:42
@DrHyde DrHyde merged commit aec695d into Ovid:genio-test-style Feb 18, 2019
@DrHyde
Copy link
Collaborator

DrHyde commented Feb 18, 2019

Merged into a different branch so I can run all the tests more easily without updating master.

Assuming all is well (and I see no reason why it won't be) I'll merge into master shortly. Thanks for your contribution.

@DrHyde
Copy link
Collaborator

DrHyde commented Feb 18, 2019

And lo, all was good, so merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants