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

[bext-sml] Adding new port of boost::sml #20851

Merged
merged 2 commits into from
Oct 27, 2021

Conversation

misirlou-tg
Copy link
Contributor

Adding new port of boost::sml

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

Copy link
Contributor

@yurybura yurybura left a comment

Choose a reason for hiding this comment

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

I think boost-ext-sml is more clear name for this port.

@misirlou-tg
Copy link
Contributor Author

I was following the example of bext-di and bext-ut, they are all a part of https://github.com/boost-ext

@yurybura
Copy link
Contributor

I was following the example of bext-di and bext-ut, they are all a part of https://github.com/boost-ext

I've just checked DI and UT documentation. Sorry, but I haven't found these examples. I've only found that the boost-ext-ut package is available from Conan Center.

@misirlou-tg
Copy link
Contributor Author

misirlou-tg commented Oct 19, 2021

Those two I brought up are already ports in vcpkg, https://github.com/microsoft/vcpkg/tree/master/ports/bext-di and https://github.com/microsoft/vcpkg/tree/master/ports/bext-ut

I don't mind changing it, I just don't know if consistency or a more readable name is better.

@yurybura
Copy link
Contributor

I see. Thank you. But I think searching by name boost will be very useful. Let's wait opinion of maintainers.
I vote to rename bext-di and bext-ut too)

@PhoebeHui PhoebeHui added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Oct 20, 2021
@PhoebeHui
Copy link
Contributor

I think the name is fine, See #16302 (review)

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Oct 20, 2021
@niclar
Copy link
Contributor

niclar commented Oct 20, 2021

I think the name is fine, See #16302 (review)

bext is a terrible prefix. you could also change the regexp

@yurybura
Copy link
Contributor

I think the name is fine, See #16302 (review)

I don't understand from the comment why boost-ext library shouldn't contain boost in the name.

@BillyONeal
Copy link
Member

I think the name is fine, See #16302 (review)

I don't understand from the comment why boost-ext library shouldn't contain boost in the name.

This library is not in boost, so it should not use boost- as a prefix. Just because the library wants to claim that it's part of boost, it isn't part of boost until the boost maintainers have accepted it.

@yurybura
Copy link
Contributor

I think the name is fine, See #16302 (review)

I don't understand from the comment why boost-ext library shouldn't contain boost in the name.

This library is not in boost, so it should not use boost- as a prefix. Just because the library wants to claim that it's part of boost, it isn't part of boost until the boost maintainers have accepted it.

In source code everybody must use boost:

#include <boost/di.hpp>
#include <boost/sml.hpp>
#include <boost/ut.hpp>

Why in the package name boost is forbidden? IMHO bext is awful prefix.

@vicroms
Copy link
Member

vicroms commented Oct 22, 2021

For the main registry we are not changing our stance on these two things:

  • Libraries not officially affiliated with Boost should not use the boost- prefix.
  • The boost-ext-XYZ prefix is not enough since it could collide with an hypothetical official ext-XYZ library.

If I recall correctly the bext- prefix was picked by a contributor to boost-ext/di and we have used that prefix for at least one other port. Therefore the argument to keep using the bext- prefix is to keep consistency. If you want to propose a new name I would suggest to consult with the boost-ext org maintainers for their opinion.

An alternative is to create a private repository that uses boost-ext- for these libraries and promote it in the community registries thread.

@misirlou-tg since you're the author of this PR. If you're ok with the current port name let me know so I can merge it.

@vicroms vicroms added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Oct 22, 2021
@niclar
Copy link
Contributor

niclar commented Oct 22, 2021

@vicroms thanks for clarifying. -On the other hand you could just move it, if the hypothetical library realizes.

@yurybura
Copy link
Contributor

@vicroms Thank you for your detailed answer.

@misirlou-tg
Copy link
Contributor Author

@misirlou-tg since you're the author of this PR. If you're ok with the current port name let me know so I can merge it.

Yes, I am OK with the name

@PhoebeHui PhoebeHui added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Oct 25, 2021
@BillyONeal BillyONeal merged commit 3a733ff into microsoft:master Oct 27, 2021
@BillyONeal
Copy link
Member

Thanks!

@misirlou-tg misirlou-tg deleted the add-boost-sml branch October 27, 2021 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants