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

Switch to Types::Serialiser booleans #37

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

Conversation

aferreira
Copy link

This change promotes type compatibility with other
CPAN serialization modules like JSON.

This is another take at #17 – see also #34

BREAKING CHANGE: removes stringification of true()
to "true" and false() to "false". As the packages
Data::MessagePack::Boolean and Types::Serialiser::Boolean
are aliased, it is not safe to include this controverse
coertion (different modules may have different ideas
on what is a useful stringification of booleans).
The "bool" and "num" (0+) overloads should work as before.

This change promotes type compatibility with other
CPAN serialization modules like JSON.

BREAKING CHANGE: removes stringification of true()
to "true" and false() to "false". As the packages
Data::MessagePack::Boolean and Types::Serialiser::Boolean
are aliased, it is not safe to include this controverse
coertion (different modules may have different ideas
on what is a useful stringification of booleans).
The "bool" and "num" (0+) overloads should work as before.
@FGasper
Copy link

FGasper commented Sep 26, 2017

@aferreira How does this compare to #34?

@aferreira
Copy link
Author

@FGasper I took a lazier route to achieve interoperability – namely instead of also supporting Types::Serialiser::Boolean, the proposed change makes "Data::MessagePack::Boolean" and "Types::Serialiser::Boolean" the same for Perl. So the assertions below will hold.

A consequence of this approach is that the required change was simpler – no need for patches to XS code or adding explanation on the tolerance of Types::Serialiser::Boolean to documentation.

use Test::More;
use Data::MessagePack::Boolean;
use Types::Serialiser;

ok(Data::MessagePack->true->isa('Data::MessagePack::Boolean'));  # old stuff
ok(Data::MessagePack->true->isa('Types::Serialiser::Boolean'));
ok(Data::MessagePack->false->isa('Data::MessagePack::Boolean'));  # old stuff
ok(Data::MessagePack->false->isa('Types::Serialiser::Boolean'));

ok(Types::Serialiser::true()->isa('Types::Serialiser::Boolean')); # old stuff
ok(Types::Serialiser::true()->isa('Data::MessagePack::Boolean'));
ok(Types::Serialiser::false()->isa('Types::Serialiser::Boolean')); # old stuff
ok(Types::Serialiser::false()->isa('Data::MessagePack::Boolean'));

done_testing;

@FGasper
Copy link

FGasper commented Sep 26, 2017

To my knowledge #34 doesn’t include any breaking changes, so I’d think that would still be a preferable solution—notwithstanding my own bias. :)

Anyway, I think it’s moot until someone else assumes stewardship of the module.

@FGasper
Copy link

FGasper commented Sep 26, 2017

In the meantime there is a (very ugly) workaround solution here:

https://metacpan.org/source/FELIPE/Net-WAMP-0.01/lib/Net/WAMP/Serialization/msgpack.pm

@foobargeez
Copy link

Thanks. Given that the owner is unresponsive, any ideas on what would be the next steps to get the PR merged to master and released to CPAN?

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.

3 participants