Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add package ipaddress #5506

Merged
merged 8 commits into from
Oct 16, 2024
Merged

add package ipaddress #5506

merged 8 commits into from
Oct 16, 2024

Conversation

Luffbee
Copy link
Contributor

@Luffbee Luffbee commented Oct 15, 2024

ipaddress

https://github.com/VladimirShaleev/ipaddress/tree/main

A library for working and manipulating IPv4/IPv6 addresses and networks in modern C++.

This cross-platfrom header-only library (for C++11 and newer) is inspired by the ipaddress API in Python, from which it derives its name. It aims to be simpler to use due to its familiar interface. However, the C++ implementation takes a different approach: it uses static polymorphism through the strategy pattern instead of dynamic polymorphism to handle differences between IP versions (IPv4 and IPv6). This design choice eliminates the overhead of dynamic calls and virtual tables. For instance, an instance of the ipv4_address class will be represented by 4 bytes.

The library leverages modern C++ features, ensuring that all IP address and network operations support constant expressions.

@SirLynix
Copy link
Member

ipaddress is a very generic name, the package should probably be called vladimirshaleev_ipaddress (similar to what vcpkg does)

std::cout << " " << subnet << std::endl;
}

constexpr auto last_subnet = net.subnets(2).back();
Copy link
Member

Choose a reason for hiding this comment

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

too large test code, please simplify it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about now?

@Luffbee
Copy link
Contributor Author

Luffbee commented Oct 16, 2024

ipaddress is a very generic name, the package should probably be called vladimirshaleev_ipaddress (similar to what vcpkg does)

renamed to the same name as vcpkg


add_versions("v1.1.0", "e5084d83ebd712210882eb6dac14ed1b9b71584dede523b35c6181e0a06375f1")

add_configs("noexcept", {description = "Disable handling cpp exception for", default = false, type = "boolean"})
Copy link
Member

Choose a reason for hiding this comment

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

noexcept -> except

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I searched similar configs, spdlog use 'noexcept' and some others use 'exceptions'. So maybe 'exceptions' is better?

@waruqi waruqi merged commit 355ff95 into xmake-io:dev Oct 16, 2024
0 of 16 checks passed
@SirLynix
Copy link
Member

I don't get it, if it's a header-only package, options are meaningless if they don't add any define to the package

@waruqi
Copy link
Member

waruqi commented Oct 16, 2024

I don't get it, if it's a header-only package, options are meaningless if they don't add any define to the package

you are right, they should be exported by package:add("defines", "xxx")

@Luffbee

https://github.com/VladimirShaleev/ipaddress/blob/fffbd6cb64df8be25de2e3d992771a0fa57bd97c/CMakeLists.txt#L29

@Luffbee
Copy link
Contributor Author

Luffbee commented Oct 16, 2024

I don't get it, if it's a header-only package, options are meaningless if they don't add any define to the package

you are right, they should be exported by package:add("defines", "xxx")

@Luffbee

https://github.com/VladimirShaleev/ipaddress/blob/fffbd6cb64df8be25de2e3d992771a0fa57bd97c/CMakeLists.txt#L29

Sorry, I thought xmake would extract these defines from cmake. I will fix it in another PR.

P.S. Is there any document for these contribution FAQs? (e.g. format, option naming, options of headeronly package). I did some searching but couldn't find any.

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