Skip to content

Conversation

@Gary-Hobson
Copy link
Contributor

Summary

error: variable 'ret' set but not used [-Werror=unused-but-set-variable]
112 | int ret;

Impact

Testing

sim

error: variable 'ret' set but not used [-Werror=unused-but-set-variable]
   112 |   int ret;

Signed-off-by: yinshengkai <yinshengkai@xiaomi.com>
@nuttxpr
Copy link

nuttxpr commented Oct 12, 2024

[Experimental Bot, please feedback here]

This PR does not meet the NuttX requirements.

Missing Information:

  • Purpose of the Change: The summary only mentions a compiler error being fixed, but doesn't explain why the variable ret was unused in the first place.
    • Was it leftover code?
    • Did the logic change, making it obsolete?
  • Impact Sections are Empty: You need to analyze and fill out all impact sections, even if the answer is "NO". Provide brief explanations when applicable.
  • Insufficient Testing Details:
    • Build Host: Specify the OS, CPU architecture, and compiler version used for building.
    • "sim" is too vague: Which simulator? QEMU? Something else? Be specific about the simulated architecture and board configuration.
    • No "Before" Logs: Without seeing the error message in the "before" logs, it's impossible to verify the fix.

To Improve this PR:

  1. Expand the Summary: Clearly explain the reason behind the unused variable and the fix.
  2. Complete the Impact Assessment: Address each impact category, providing "NO" or a brief explanation if relevant.
  3. Provide Detailed Testing Information: Include specifics about your build host, the exact simulator configuration, and the relevant "before/after" log snippets demonstrating the error and its resolution.

@xiaoxiang781216
Copy link
Contributor

let's ignore ci check error which is fixed by #2712 to avoid wasting github resources.

@xiaoxiang781216 xiaoxiang781216 merged commit dbef7b5 into apache:master Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants