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

Refactoring and execption message improvements #66

Merged
merged 10 commits into from May 24, 2015

Conversation

Projects
None yet
2 participants
@bugdebugger
Contributor

bugdebugger commented May 24, 2015

This pull request builds upon pull #65 and

  • fixes a bug in the octal formatting of permission mask parameters
  • refactors a few duplicated code pieces into helper functions
  • improves the octal display of permission masks

This pull-request is part of the 2015 CPAN PR Challenge

bugdebugger added some commits May 24, 2015

Add _beautify_arguments helper function
Extracted custom arguments formatting from default formatter.
The new chmod formatter also needs this functionality in order to
keep the message similar to before
Add _octalize_number helper
Duplicated number formatting code extracted into a helper function
Fix "octalize" regex to match single digit number
Previously the regex failed to match single digit numbers.
Thus the numbers (decimal) 8 and 9 did not get formatted in octal.
Tests now check for this edge case.

Also t/dbmopen.t now declares an explicit number of tests
Use the '#' flag to ensure leading 0 for octal
Using the '#' sprintf-flag made the "octalization" regex simpler.
Now the regex only needs to check if the parameter is a whole number
Change _octalize_number formatting
Now the number is always formatted to look like a proper permissions mask
Add _trim_package_name helper function
Extracted code to trim package of dying sub into helper function
Fix error for Perl <5.14
The "/r" modifier is only available in Perl >= 5.14
@nthykier

This comment has been minimized.

Show comment
Hide comment
@nthykier

nthykier May 24, 2015

Collaborator

Hi,

Thanks for working on this.

Unfortunately, it seems like the last commit introduces a test failure with perl 5.12 and older, which means we cannot merge it as-is.

Collaborator

nthykier commented May 24, 2015

Hi,

Thanks for working on this.

Unfortunately, it seems like the last commit introduces a test failure with perl 5.12 and older, which means we cannot merge it as-is.

@nthykier

This comment has been minimized.

Show comment
Hide comment
@nthykier

nthykier May 24, 2015

The "/r" flag is only available since Perl 5.14. I believe the following backwards compatible alternative should work just fine

sub _trim_package_name {
    my ($self, $name) = @_;
    $name =~ s/.*:://;
    return $name;
}

nthykier commented on lib/autodie/exception.pm in 2662d92 May 24, 2015

The "/r" flag is only available since Perl 5.14. I believe the following backwards compatible alternative should work just fine

sub _trim_package_name {
    my ($self, $name) = @_;
    $name =~ s/.*:://;
    return $name;
}

nthykier added a commit that referenced this pull request May 24, 2015

Merge pull request #66 from bugdebugger/refactoring
Refactoring and execption message improvements

@nthykier nthykier merged commit 623a39b into pjf:master May 24, 2015

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@nthykier

nthykier May 24, 2015

Collaborator

Looks good, thanks. :)

Collaborator

nthykier commented May 24, 2015

Looks good, thanks. :)

@bugdebugger bugdebugger deleted the bugdebugger:refactoring branch May 24, 2015

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