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 #338, Add MISRA Addons for cppcheck #339

Conversation

ArielSAdamsNASA
Copy link
Contributor

@ArielSAdamsNASA ArielSAdamsNASA commented Aug 18, 2021

Checklist (Please check before submitting)

Describe the contribution
Fixes #338

Testing performed
Tested on forked repo: https://github.com/ArielSAdamsNASA/cFS-JSF-Rules/actions/runs/1143235812

image

Expected behavior changes
Runs MISRA rules for bundle, cfe, osal, and psp. Has a job for regular "clean" cppcheck and misra cppcheck.

Additional context
Future work includes:

  • Running MISRA rules for the bundle, cfe, osal, and psp.
  • Showing errors in the check for errors step

Contributor Info - All information REQUIRED for consideration of pull request
Ariel Adams, ASRC Federal

Copy link
Contributor

@skliper skliper left a comment

Choose a reason for hiding this comment

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

I don't think we are allowed to publish MISRA rules, also it looks like the check is just running against misra-test.c. I recommend starting with a local report and doing some analysis there prior to pushing it into CI...

@ArielSAdamsNASA
Copy link
Contributor Author

I don't think we are allowed to publish MISRA rules, also it looks like the check is just running against misra-test.c. I recommend starting with a local report and doing some analysis there prior to pushing it into CI...

@skliper See new changes. Created a separate job for misra rules. Ran misra analysis against bundle, cfe, osal, and psp instead of misra-test.c.

@ArielSAdamsNASA ArielSAdamsNASA marked this pull request as ready for review August 18, 2021 13:18
@ArielSAdamsNASA ArielSAdamsNASA added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Aug 18, 2021
@skliper
Copy link
Contributor

skliper commented Aug 18, 2021

It's great work, I'm just concerned it may be a little premature to put into CI to run on every push from a project management perspective since we don't really have a plan in place with what to do with the actual results within the repo management process. Maybe make into an action that just gets triggered manually?

@astrogeco astrogeco removed the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Aug 18, 2021
@ArielSAdamsNASA
Copy link
Contributor Author

Updated to trigger workflow manually.

@skliper
Copy link
Contributor

skliper commented Aug 18, 2021

Sorry, I meant trigger the MISRA check as manual. The normal static analysis should still be in the normal push/merge flow since we do need to enforce it.

@ArielSAdamsNASA
Copy link
Contributor Author

Sorry, I meant trigger the MISRA check as manual. The normal static analysis should still be in the normal push/merge flow since we do need to enforce it.

So have a separate workflow for the misra analysis that runs manually?

@ArielSAdamsNASA ArielSAdamsNASA force-pushed the fix-338-misra-addon-cppcheck branch 2 times, most recently from 37d2d78 to 33e0c1e Compare August 18, 2021 16:24
@astrogeco astrogeco added the CCB:Approved Indicates code review and approval by community CCB label Aug 21, 2021
@astrogeco astrogeco changed the base branch from main to integration-candidate August 21, 2021 00:18
@astrogeco astrogeco merged commit 5678051 into nasa:integration-candidate Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MISRA Addon for cppcheck static analysis
4 participants