Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

two really trivial changes #1

Merged
merged 4 commits into from Nov 22, 2011

Conversation

Projects
None yet
2 participants
Contributor

esessoms commented Oct 18, 2011

I admit to being a bit nonplussed when I saw your announcement on the 0MQ list (why another library with the exact same name, etc.) but I have to admit I like your implementation better. It's just Lisp-ier. :)

Anyway, in integrating it into my project, I discovered one docstring error, and I'm of the opinion that a minor change to the poll interface makes things even "more Lisp-ier".

BTW, I'm running on Gentoo 32-bit and 64-bit with zeromq-2.1.10. No problems. I'll probably be testing on Lion later today.

Thanks, this is nice work.

Owner

galdor commented Oct 18, 2011

Ok for the first patch.

Concerning the second patch:

  • Why do you export event-types ? You don't need the foreign type: (zmq:poll-item-event-signaled-p item :pollin).
  • You're right, allowing more than one event is nice, but the function should be renamed to poll-item-events-signaled-p.

Could you make the modifications (including the tests and examples, and updating the docstring) ? I'll release a new minor version after that.

Thank you!

Contributor

esessoms commented Oct 18, 2011

I exported it in the first commit, because I needed to construct an composite event to pass to poll-item-event-signaled-p. Then when I realized it would be more natural to have the function take a list of events, I no longer needed the export and removed it in the second commit. So, it was a momentary confusion.

But, yes, I will make all the required changes and get back to you. Should be less than 24 hours.

Contributor

esessoms commented Nov 21, 2011

Updated tests and docstring. Didn't see an example using polling.

Owner

galdor commented Nov 21, 2011

I'll have a look tomorrow evening, thank you!

galdor added a commit that referenced this pull request Nov 22, 2011

Merge pull request #1 from esessoms/master
two really trivial changes

@galdor galdor merged commit e9a10f0 into galdor:master Nov 22, 2011

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