On Tue, Dec 21, 2010 at 8:49 AM, Scott <alcoholi...@gmail.com> wrote:
> Well... after a little testing, I find that it's exploding when I try
> to do a connect("epgm://en1;293.192.192.192:5000"), so we're not quite
> there yet! At the same time openpgm does compile and complete its
> tests.
>
> However, I'd still appreciate some feedback regarding the format of
> the changes, or if folks think I should make an OSX branch?
>
Hi Scott,
overall the patch looks good. Following things came into mind when
skimming through it:
1) rather than having a switch to checkout directly from openpgm svn
it might make more sense to add --with-pgm-source=/path/to switch that
would allow specifying a path in the filesystem. This approach would
allow testing local changes a lot more easily and doesn't break if
openpgm decides to change repository location in the future. What do
you think?
2) The automake conditional uses ON_DARWIN but the code uses
ZMQ_HAVE_OSX. I think it would be safe to use ON_OSX in automake as
well.
3) PGMEXTRASOURCE should be PGM_EXTRA_SOURCE or even AC_ZMQ_PGM_EXTRA_SOURCE.
4) libpgm_diff_flags inside ON_DARWIN should be one per line.
I think the plan is to add support for building libzmq with openpgm on
Solaris and FreeBSD as well. It might even make sense to split
operating system specifics into their respective automake files and
include based on operating system. It might be a bit cleaner approach
than using conditionals. I am not sure whether this is possible just
by doing AC_CONFIG_FILES([foreign/Makefile.linux.am]) or something
similar.
Can you send the patch as attachment as well so that I can properly
test it? It seems that my mail client doesn't handle inline patches
very well.
--
Mikko Koppanen
_______________________________________________
zeromq-dev mailing list
zeromq-dev@lists.zeromq.org
http://lists.zeromq.org/mailman/listinfo/zeromq-dev
