-
Notifications
You must be signed in to change notification settings - Fork 351
Update project to require C++17 #1266
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
Conversation
@antoninbas please review my PR |
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.
I wonder if the reference to C++11 in the README should be updated?
I see multiple CI failures for your PR.
@@ -0,0 +1,35 @@ | |||
# ============================================================================= |
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.
I think you also need https://www.gnu.org/software/autoconf-archive/ax_cxx_compile_stdcxx.html in this m4 directory?
m4/ax_cxx_compile_stdcxx.m4
Outdated
|
||
#endif // __cplusplus < 202002L && !defined _MSC_VER | ||
|
||
]]) |
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.
file is missing a trailing newline character (applies to all the other new files as well)
I'm not sure about the exact cause of the CI failures at the moment. Could you please provide more details about the specific failures? I'd be happy to investigate further and work on resolving these issues. Any additional information or logs you can share as it will help me address the problems more effectively |
I am pretty sure that the detailed log output of all CI tests, whether they are failing or successful, are publicly available to anyone. If you look at this PR on github.com at this link #1266 and scroll down to the bottom, near the bottom you should see a list of tests with names like these, and a "Details" link to the right of each one:
as well as some tests that have been successful. |
@antoninbas Are there any additional changes or updates that need? |
@OsamaRab3 At least one commit is not signed, so the DCO check is failing. I'd suggest squashing all 4 commits, and signing the resulting commit. |
@antoninbas I reviewed the DCO check failure details and noticed that it was due to missing "Signed-off-by" lines in the commit messages. Following the instructions provided in the error message, I have rebased the branch and added the necessary "Signed-off-by" lines to each commit. |
m4/ax_cxx_compile_stdcxx_17.m4
Outdated
#serial 2 | ||
|
||
AX_REQUIRE_DEFINED([AX_CXX_COMPILE_STDCXX]) | ||
AC_DEFUN([AX_CXX_COMPILE_STDCXX_17], [AX_CXX_COMPILE_STDCXX([17], [$1], [$2])]) |
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.
missing newline character
#serial 2 | ||
|
||
AX_REQUIRE_DEFINED([AX_CXX_COMPILE_STDCXX]) | ||
AC_DEFUN([AX_CXX_COMPILE_STDCXX_17], [AX_CXX_COMPILE_STDCXX([17], [$1], [$2])]) |
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.
ditto
Signed-off-by: osama <osrab3@gmail.com>
Signed-off-by: osama <osrab3@gmail.com>
Signed-off-by: osama <osrab3@gmail.com>
Signed-off-by: osama <osrab3@gmail.com>
Signed-off-by: osama <osrab3@gmail.com>
@antoninbas any thing else? |
enforce -std=c++17 support using AX_CXX_COMPILE_STDCXX_17