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.

Sign up for GitHub

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

Jump to bottom

Add 5.6 support (without unicode features) #8

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add 5.6 support (without unicode features) #8

wants to merge 3 commits into from

Conversation

rurban
Copy link

@rurban rurban commented Jul 19, 2013

No description provided.

Reini Urban added 3 commits July 19, 2013 11:30
wrong L<> pod. L<name|link>
ee25e88 
fixes t/98_pod.t
1..29
ok 1 - POD test for blib/lib/DBM/Deep.pm (no pod)
not ok 2 - POD test for blib/lib/DBM/Deep.pod
  Failed test 'POD test for blib/lib/DBM/Deep.pod'
  at /usr/local/lib/perl5/site_perl/5.18.0/Test/Pod.pm line 186.
blib/lib/DBM/Deep.pod (52): alternative text '/db_version' contains non-escaped | or /
better .gitignore
0171be7
support 5.6 without unicode features
87fe951
@cpansprout
Copy link
Collaborator

By avoiding ‘use constant’, you are preventing unused code paths from being optimised away, creating a run-time speed hit. The most recent constant.pm release might not work in 5.6, but 5.6 already comes with its own constant.pm, which should work. Apart from that, I think the patch is fine.

@robkinyon
Copy link
Owner

I would like the following two changes:

  1. Please add "use constant" back in per cpansprout's suggestion
  2. Please add a CAN_UNICODE (or somesuch name) constant that is the check for $] >= 5.008003. The first is that I don't like that check just randomly scattered about. The second is that we may expand the can_unicode check to include other criteria. Self-documenting code, where possible, is ideal.

Once you do those two things, I will merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rurban @cpansprout @robkinyon