Skip to content

Conversation

OsamaRab3
Copy link
Contributor

@OsamaRab3 OsamaRab3 commented Aug 25, 2024

enforce -std=c++17 support using AX_CXX_COMPILE_STDCXX_17

@OsamaRab3 OsamaRab3 marked this pull request as draft August 25, 2024 13:29
@OsamaRab3 OsamaRab3 marked this pull request as ready for review August 25, 2024 14:09
@OsamaRab3
Copy link
Contributor Author

@antoninbas please review my PR

Copy link
Member

@antoninbas antoninbas left a 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 @@
# =============================================================================
Copy link
Member

Choose a reason for hiding this comment

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


#endif // __cplusplus < 202002L && !defined _MSC_VER

]])
Copy link
Member

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)

@OsamaRab3
Copy link
Contributor Author

I see multiple CI failures for your PR.

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

@jafingerhut
Copy link
Contributor

I see multiple CI failures for your PR.

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:

  • Test / test (clang++-10, clang-10) (pull_request). Failing after 12m
  • Test / test-ubuntu22 (pull_request) Failing after 1m

as well as some tests that have been successful.

@OsamaRab3
Copy link
Contributor Author

@antoninbas Are there any additional changes or updates that need?

@antoninbas
Copy link
Member

@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.

@OsamaRab3
Copy link
Contributor Author

@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.
Please let me know if there are any additional steps I should take or if further adjustments are needed

#serial 2

AX_REQUIRE_DEFINED([AX_CXX_COMPILE_STDCXX])
AC_DEFUN([AX_CXX_COMPILE_STDCXX_17], [AX_CXX_COMPILE_STDCXX([17], [$1], [$2])])
Copy link
Member

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])])
Copy link
Member

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>
@OsamaRab3
Copy link
Contributor Author

@antoninbas any thing else?

@antoninbas antoninbas merged commit 359aeea into p4lang:main Sep 3, 2024
7 checks passed
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