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

sim/minmea: Fix defconfig warning #13534

Conversation

JianyuWang0623
Copy link
Contributor

Summary

Warning/Error:

$ ./tools/configure.sh -l sim:minmea
$ make defconfig
$ make savedefconfig
$ diff defconfig boards/sim/sim/sim/configs/minmea/defconfig
38d37
< CONFIG_GNSSUTILS_MINMEA_LIB=y
39a39
> CONFIG_GNSSUTILS_MINMEA_LIB=y

Impact

sim:minmea

Testing

./tools/configure.sh -l sim:minmea

Warning/Error:
  $ ./tools/configure.sh -l sim:minmea
  $ make defconfig
  $ make savedefconfig
  $ diff defconfig boards/sim/sim/sim/configs/minmea/defconfig
  38d37
  < CONFIG_GNSSUTILS_MINMEA_LIB=y
  39a39
  > CONFIG_GNSSUTILS_MINMEA_LIB=y

Signed-off-by: wangjianyu3 <wangjianyu3@xiaomi.com>
@lupyuen
Copy link
Member

lupyuen commented Sep 19, 2024

[Experimental Bot, please feedback here]

This PR does not meet the NuttX requirements. Here's why:

  • Missing Information in Summary: The summary only provides an error message. It needs to explain:
    • Why the change is necessary (What problem does the error indicate? Why is the config option duplicated?)
    • What the change does to fix the problem.
    • How the change works.
  • Insufficient Impact Assessment:
    • The impact section only lists "sim:minmea". It needs to clarify what impact the change has on the user, build process, hardware, documentation, security, and compatibility. For example, does this fix a bug? Does it add a new feature?
  • Incomplete Testing Information:
    • Build Host: Provide details about the OS, CPU architecture, and compiler used for testing.
    • Target: Specify the simulator version and configuration used.
    • Logs: The testing logs are empty. Include relevant logs from before and after the change to demonstrate the issue and the fix.

To improve this PR:

  1. Expand the Summary: Explain the problem, solution, and how it works.
  2. Complete the Impact Assessment: Address all the impact points with specific details.
  3. Provide Detailed Testing Information: Include the Build Host, Target details, and relevant testing logs demonstrating the problem and the fix.

@xiaoxiang781216 xiaoxiang781216 merged commit 012dcc4 into apache:master Sep 19, 2024
13 of 28 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