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
Minor optimisation to ATCmdParser unit test code #12291
Conversation
1. replace 'char buf[8]; memset(buf, 0, 8);' with 'char buf[8] = {0};'.
@DavidLin1577, thank you for your changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your change.
The first buf
initialisation is not required but does not harm.
The second initialisation is required by the test. Although the memset
implementation was fine, it is better to zero initialise the array. (There was a recent change to increase buf
size and memset
had to be modified as well).
Could you update :
- the PR description to explain what you changed and why, see guidelines here https://os.mbed.com/docs/mbed-os/v5.15/contributing/workflow.html#guidelines-for-github-pull-requests
- the Test Results section to indicate that test is
Covered by existing mbed-os test
.
Thanks so much for evedon: |
@DavidLin1577 Re-worded the title and PR description for readability. PR approved. |
CI started |
Test run: SUCCESSSummary: 1 of 1 test jobs passed |
This PR does not contain release version label after merging. |
It looks like we shall first mark the version the merge. Timing here might be involved. As I tested this on another PR that I marked version first then merged, no release review required. This was done other way around. Anyway, will report this to mergify |
Summary of changes
This is a minor optimisation to the unit test code:
memset
. The code is more readable and portable.Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers