bgamari / nanomsg-haskell

Haskell bindings to nanomsg
MIT License
25 stars 12 forks source link

Moved to Network.Nanomsg and use Tasty and 0MQ4 #2

Closed jcristovao closed 10 years ago

jcristovao commented 10 years ago

Hi, I propose some changes, but these are mostly design choices, so please take them with a grain of salt:

Tests and benchmarks are passing and running, respectively.

What do you think?

ghost commented 10 years ago

Hi,

I think this package should be under the Network hierarchy of modules, or alternatively System (as 0MQ). Personally I think Network makes more sense, but any of those seem preferable to just Nanomsg.

Why? What is wrong with the current name, what is gained by adding a prefix? I've seen module naming discussed elsewhere[1][2], and don't remember coming across a good reason for what you're suggesting.

By all means try to convince me otherwise. If you're primarily concerned with taxonomy, I think using the category field in the cabal file would be better. How about tagging the package as Network and/or MQ/messaging?

Test-framework seems mostly dead. While there seems to be an active fork, tasty seems to be more actively maintained.

My impression is that test-framework is alive, mature and maintained by some very reputable developer(s). I'm not opposed to switching, though. Tasty seems to pull in less dependencies for me, and builds significantly faster.

Replaced zeromq3-haskel with zeromq4-haskell

Ok, I guess that warrants a new release, and we might as well go with Tasty. Remember dropping the version number back down to 0.2.2 or something unless you manage to convince me on the module naming issue.

jcristovao commented 10 years ago

Hi,

I actually wasn't aware of those discussions, quite interesting, thanks! Nevertheless, I still find myself on the other side of the baricade: I cannot see enough good reasons to drop the prefix. The category in the cabal file is used in hackage alright, but I do find that their presence on the import makes it clearer for me to quickly (and visually) group the imports, and navigating the haddock documentation for a new package makes it very clear what the package does, and where to find the functions I'm interested. A flat structure makes this much more difficult...

Besides, most of the main Haskell authors (E. Kmett, BOS, etc) keep using the prefix hierarchy... When I see a module without a prefix, I usually take some extra care to double check the code of the module.

But nevertheless, this is your package, and ultimately the decision is up to you, of course :) I can revert those changes if you want. I only raised the version number to 0.3 to reflect the namespace change, of course.

Regarding tasty vs test-framework I don't really have a strong opinion about either, I use hspec. This is based mainly on watching the community, and the feeling that Tasty is more active.

Replaced zeromq3-haskel with zeromq4-haskell Ok, I guess that warrants a new release, and we might as well go with Tasty.

Just out of curiosity, in the previous benchmarks how did nanomsg fair against zeromq3? Somewhat naively, I was expecting nanomsg to be faster, but that seems to be hardly the case against zeromq4.

Cheers, João

ghost commented 10 years ago

Hey João,

I'm not convinced on renaming the module, sorry. I'd like to keep that as-is.

Regarding tasty vs test-framework I don't really have a strong opinion about either, I use hspec.

Hmm.. I will apply this patch if you drop the namespace changes, but after that sentence I'm thinking it'd almost be change just for the sake of change. I have no strong opinion myself. Tasty does install a bit faster, but it's also less mature.

Do you think still think we should go ahead with the patch? I could also just apply and release my own zmq4-update, but I've been reluctant to do that as there is probably still a lot of zmq3-users out there.

Just out of curiosity, in the previous benchmarks how did nanomsg fair against zeromq3?

It's been a while since I switched to zmq4, but iirc benchmarks were pretty similar to now. Nano is even on some tests and significantly slower on others. It works for me though, and I really prefer the licensing.

I've done some rudimentary debugging (and the particular issue I was looking into was also there in my native test), but that doesn't mean this binding isn't horribly broken in some way. I'd love to see a native benchmark as a reference point, but haven't come across one.

jcristovao commented 10 years ago

Hey again, I concede that the pull request was a little hasty... So, point by point:

1) I'm not convinced on renaming the module, sorry.

Ok :)

2) tasty vs test-framework

To be honest, when I've changed it to tasty I was not aware that test-framework had been patched and updated by BOS. That changes the need to replace it with tasty, and thus I agree: lets keep it (test-framework).

3) zmq3 vs zmq4

This one I do think its the one that may still be worth doing, but the question is when. Being a user of Arch Linux myself, i have only zmq4 available in the repositories. But I guess a lot of debian users may still have zmq3.

Perhaps a cabal flag to select either one? On the other hand, zmq4 is probably here to stay, so perhaps it makes sense to upgrade...

jcristovao commented 10 years ago

Ok, I've uploaded to my repo the zmq4 changes alone, with a bump to 0.2.2. If you wan't I can send a new pull request. Cheers