-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Added formattable concept #3974
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. It looks good but please address the inline comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add a test case to base-test?
I used I am not familiar with the CI test setup, but do we already have C++20 coverage? |
@vitaut, Maybe makes sense to reintroduced tests for g++-11 and C++20 into CI? It was removed in this commit: 0b5287f#diff-21364b2e6fae1f2875cee1ab3daefb0685403687eaf8bc32b5c6eacda351c9d3 |
I don't think the tests need to be exhaustive. It's enough to cover important categories of formattable types. We don't need to test every type that has a user-defined formatter.
We do.
I removed g++-11 because its installation was broken but a PR to reintroduce it (or another version that supports C++20) would be welcome. |
I agree, but I would have expected some |
No description provided.