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

{Bp-2655} examples: fix noteprint compile error #2770

Merged
merged 13 commits into from
Oct 25, 2024

Conversation

jerpelea
Copy link
Contributor

@jerpelea jerpelea commented Oct 25, 2024

Summary

fix noteprint compile error
The sched_note_string/bprintf/dump interfaces have been removed and replaced with printf and event

Impact

RELEASE

Testing

CI

Note
includes
#2596
#2614
#2650
#2677
#2681

Gary-Hobson and others added 2 commits October 25, 2024 09:35
The sched_note_string/bprintf/dump interfaces have been removed and replaced with printf and event

Signed-off-by: yinshengkai <[email protected]>
noteprintf_main.c:52:7: warning: unused variable 'binary' [-Wunused-variable]
   52 |     } binary =

Fix typo of sched.

Signed-off-by: Neo Xu <[email protected]>
@nuttxpr
Copy link

nuttxpr commented Oct 25, 2024

[Experimental Bot, please feedback here]

This Pull Request appears to meet the basic NuttX requirements.

Here's a breakdown based on the provided information:

Strengths:

  • Summary: Clearly states the purpose of the PR ("fix noteprint compile error") and provides a concise explanation of the change ("sched_note_string/bprintf/dump interfaces replaced with printf and event").
  • Impact: Indicates the PR is a bug fix affecting the RELEASE (likely meaning it's a critical fix).
  • Testing: States that CI (Continuous Integration) was used for testing. This suggests automated tests were run.
  • Cross-referencing: Links a related NuttX Apps Pull Request, which is good practice for related changes.

Areas for Potential Improvement:

  • Impact (Specificity): While "RELEASE" suggests importance, a more detailed description of the impact would be helpful. For example:
    • What functionality was broken before this fix?
    • Which specific systems/configurations were affected by the compile error?
  • Testing (Details): While mentioning "CI" is a good start, providing a summary of the specific tests run (even if just the names or categories) would strengthen the PR.
  • Testing (Logs): The template requests "Testing logs before change" and "Testing logs after change." While you might not want to paste full logs into the PR description, consider:
    • Attaching log files to the PR.
    • Providing snippets of the most relevant log entries.
    • Linking to external log storage if available.

Recommendation:

While the PR seems to meet the basic requirements, adding more details in the "Impact" and "Testing" sections will provide reviewers with a better understanding of the changes and their validation.

Gary-Hobson and others added 11 commits October 25, 2024 13:55
gcov.c:61:6: warning: 'path' may be used uninitialized [-Wmaybe-uninitialized]
   61 |   if (path == NULL || access(path, F_OK) != 0 || atoi(strip) <= 0)
      |      ^
gcov.c: In function 'gcov_main':
gcov.c:79:19: note: 'path' was declared here
   79 |   FAR const char *path;

Signed-off-by: wangmingrong1 <[email protected]>
1. struct rpmsg_ping_s ack change to cmd;
2. Update the help descrption based on the new cmd
   meaning;

Signed-off-by: Yongrong Wang <[email protected]>
Signed-off-by: Bowen Wang <[email protected]>
nsh_syscmds.c: In function ‘cmd_rpmsg_once’:
nsh_syscmds.c:567:13: error: ‘RPMSGIOC_PANIC’ undeclared (first use in this function)
  567 |       cmd = RPMSGIOC_PANIC;
      |             ^~~~~~~~~~~~~~
nsh_syscmds.c:567:13: note: each undeclared identifier is reported only once for each function it appears in
nsh_syscmds.c:571:13: error: ‘RPMSGIOC_DUMP’ undeclared (first use in this function); did you mean ‘FIOC_DUMP’?
  571 |       cmd = RPMSGIOC_DUMP;

Signed-off-by: Bowen Wang <[email protected]>
from the cmocka.h:
 * These headers or their equivalents MUST be included prior to including
 * this header file.
 * @code
 * #include <stdarg.h>
 * #include <stddef.h>
 * #include <stdint.h>
 * #include <setjmp.h>
 * @Endcode

Signed-off-by: dulibo1 <[email protected]>
Signed-off-by: buxiasen <[email protected]>
@github-actions github-actions bot added Size: M and removed Size: S labels Oct 25, 2024
Copy link

@cederom cederom left a comment

Choose a reason for hiding this comment

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

Thank you @jerpelea :-)

@jerpelea
Copy link
Contributor Author

jerpelea commented Oct 25, 2024

@lupyuen

the CI does not use the right branch
modified: boards/arm/mps/mps3-an547/configs/ap/defconfig
modified: boards/arm/mps/mps3-an547/configs/bl/defconfig

we do not have those configs in releases/12.7

@xiaoxiang781216 @cederom please ignore the errors and merge the change ASAP

@cederom cederom merged commit a2e7624 into apache:releases/12.7 Oct 25, 2024
18 of 28 checks passed
@jerpelea jerpelea deleted the bp-2655 branch October 25, 2024 14:13
@lupyuen
Copy link
Member

lupyuen commented Oct 26, 2024

the CI does not use the right branch

Thanks @jerpelea we're tracking the issue here:

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.

10 participants