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

nuttx/arch: Remove GCCVER and add compilation options directly. #13720

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

cuiziwei1
Copy link
Contributor

Summary

Remove the judgment of GCCVER and add compilation options directly. If there is a problem that the compiler cannot recognize the compilation options, add it separately.

Impact

Testing

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: S The size of the change in this PR is small labels Sep 29, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 29, 2024

[Experimental Bot, please feedback here]

This PR appears to be missing critical information.

Here's a breakdown based on the provided template:

Summary:

  • What change is necessary: While it mentions removing "GCCVER judgment" and adding compilation options, it lacks specifics. What exact judgment is being removed? What specific compilation options are being added and for what purpose?
  • What functional part of the code is being changed: This is entirely missing. Which files, modules, or functionalities are affected by this change?
  • How does the change exactly work: Again, lacks specifics. How will the code function differently after this change?
  • Related Issues: Not provided. Are there any related issues in the NuttX or NuttX Apps repositories?

Impact:

  • All sections are empty. This section is crucial to understand the consequences of the PR. Provide details on how the change affects users, the build process, hardware, documentation, security, compatibility, and any other relevant factors.

Testing:

  • Incomplete: While it mentions testing, it doesn't provide actual details.
    • Build Host(s): List the operating systems, CPUs, and compilers used for testing.
    • Target(s): Specify the architectures, boards, and configurations used for testing.
    • Testing logs: Include actual logs from before and after the change to demonstrate the impact and correctness of the modification.

To improve this PR:

  1. Provide specific details in the Summary section about the changes made, their purpose, and the code sections involved.
  2. Thoroughly complete the Impact section, addressing each point with concrete information and explanations.
  3. Include comprehensive testing details: List all build hosts and targets, and provide actual testing logs demonstrating the change's effects.

By addressing these points, the PR will be much clearer, allowing reviewers to understand the changes and their implications effectively.

@xiaoxiang781216
Copy link
Contributor

@cuiziwei1 remove the merge patch from pr

@xiaoxiang781216 xiaoxiang781216 merged commit 394a967 into apache:master Oct 1, 2024
29 checks passed
@lupyuen
Copy link
Member

lupyuen commented Oct 2, 2024

@cuiziwei1 Please check the comments here: #13776

Older versions of GCC seem to be failing, we might need a workaround.

@cuiziwei1 cuiziwei1 deleted the arch_remove_gccver branch October 18, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants