search

uperl / Alien-Librdkafka

alien wrapper to build and access librdkafka from perl
2 stars 2 forks source link

Consider testing with XS instead of FFI #3

Closed plicease closed 5 years ago

plicease commented 5 years ago

I noticed that your .t file tests the system or built library using FFI::Platypus, which is great :+1: as the author of Platypus I think that it is great that you are using it. However, the only module that uses this Alien is using it via XS. A more reliable test would test using XS, the recommended way to do this in the Alien::Build/Alien::base system is to use Test::Alien, which allows you to build a toy XS and interrogate it to verify that it works, in a very similar way to the way the XS module will end up using it. I have found that this covers a lot of corner cases that a test that doesn't test using XS cannot. If you intend on writing FFI bindings for this Alien down the road it might make sense to keep the FFI test :)

trinitum commented 5 years ago

Hi, thank you for the PR and for this report. I have to say I don't have interest or time to continue maintaining this module, so unfortunately it's not likely to be fixed.

plicease commented 5 years ago

I note now that you've added ADOPTME as co-maint 👍 If you want to make me (PLICEASE) first-come or co-maint on this module I can do the appropriate maintenance and make a release.

trinitum commented 5 years ago

Would you like me to transfer repo as well?

plicease commented 5 years ago

yes please.

trinitum commented 5 years ago

Ownership is yours, how's about Kafka::Librd that consumes this package?

trinitum commented 5 years ago

you need to delete the fork apparently before I can do the transfer

plicease commented 5 years ago

done. Thanks,.

plicease commented 5 years ago

I'll take Kafka::Librd if you don't want to maintain it either.

trinitum commented 5 years ago

Great, thank you!