Skip to content

Conversation

@lgeiger
Copy link
Member

@lgeiger lgeiger commented Nov 4, 2016

This also simplifies the build script and move the essential stuff to cross platform js.
It will make it possible to build libzmq from source on windows if someone writes a batch script handling the build.

'libraries': [
'<(PRODUCT_DIR)/../../windows/lib/libzmq',
'ws2_32.lib',
'iphlpapi'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given I have no background on this, it looks like someone fell asleep on the keyboard to name this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol. IP Helper API. https://msdn.microsoft.com/en-us/library/windows/desktop/aa366073(v=vs.85).aspx

A good example of how adding an 'e' to make a human readable name would have been a solid idea. iphelpapi would have been better than iphlpapi.

Overall, I think the addition here is fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it's needed otherwise the build would fail

@rgbkrk
Copy link
Member

rgbkrk commented Nov 4, 2016

Im glad this leaves room for building from source on Windows.

@willingc
Copy link
Contributor

willingc commented Nov 4, 2016

@lgeiger Wow! Lots of good work here. I didn't do a line-by-line review rather a quick scan. I would say merge it.

@lgeiger
Copy link
Member Author

lgeiger commented Nov 4, 2016

@willingc zmq.h and zmq_utils.h are just copied from the latest release of libzmq so nothing interesting there 😄 .

@willingc
Copy link
Contributor

willingc commented Nov 4, 2016

💯 to humility @lgeiger.

@rgbkrk rgbkrk merged commit 4e7761f into zeromq:master Nov 4, 2016
@lgeiger lgeiger deleted the libzmq-4.2.0 branch November 4, 2016 23:39
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