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

Fix #1428, Refactor error handling for EVS_GetApplicationInfo() #2212

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thnkslprpt
Copy link
Contributor

@thnkslprpt thnkslprpt commented Nov 16, 2022

Checklist

Describe the contribution

The function prototype isn't strictly required, given the placement of the function definition, but is probably worth having to improve clarity and ease future maintenance.

Can also be implemented as an if/else if/else block (see below). No strong preference but I've left it as a switch for now.
Screenshot 2022-11-16 14 29 58

Testing performed
GitHub CI actions (incl. Build + Run, Unit Tests etc.) all passing successfully.
Tested locally as well and confirmed coverage of all new/amended lines of code via LCOV report.

Expected behavior changes
No impact on behavior. The same events will occur in the same circumstances.

System(s) tested on
Intel(R) Celeron(R) N4100 CPU @ 1.10GHz x86_64
Debian GNU/Linux 11 (bullseye)
Current main branch cFS bundle.
cFE v7.0.0-rc4+dev205

Contributor Info
Avi Weiss @thnkslprpt

@thnkslprpt
Copy link
Contributor Author

thnkslprpt commented Nov 16, 2022

Question:
Does this require new tests, or is the coverage coming through from the calling functions satisfactory?

* event from the calling function.
*
*-----------------------------------------------------------------*/
void CFE_EVS_SendEventForGetAppInfoError(int32 Status, const char *LocalName, uint16 CommandCode)

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
@thnkslprpt thnkslprpt marked this pull request as ready for review January 23, 2023 23:15
@thnkslprpt thnkslprpt force-pushed the fix-1428-refactor-error-handling-for-GetAppInfo branch 2 times, most recently from f3eed39 to d624e79 Compare March 31, 2023 04:13
@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration coding-standard. As part of the setup process, we have scanned this repository and found 1906 existing alerts. Please check the repository Security tab to see all alerts.

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration security. As part of the setup process, we have scanned this repository and found 30 existing alerts. Please check the repository Security tab to see all alerts.

@thnkslprpt thnkslprpt force-pushed the fix-1428-refactor-error-handling-for-GetAppInfo branch from d624e79 to 8679507 Compare March 23, 2024 01:38
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.

Refactor handling of EVS_GetApplicationInfo return to eliminate duplication
1 participant