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

Adds Github Code Scanning (CodeQL) #43

Merged
merged 5 commits into from May 17, 2024

Conversation

AlexandruAntonescuKeysight
Copy link
Contributor

Summary

This PR adds a workflow which enables Github Code Scanning (the engine used is CodeQL).
It provides a suite of security queries that are run on every push and pull request on main branch. The results are visible in the security tab. This also represents a check in a PR context.
The scan is being done on python and C++ code (the C++ code needs a build step; I've built et_feeder as shared lib).

Example (the errors also appear as comments in the PR):

image

Test Plan

The testing was done by adding known bad lines of code to see if the problems are identified. An example could be seen in the picture above.

Copy link

github-actions bot commented Apr 30, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@TaekyungHeo
Copy link
Contributor

Thanks, @AlexandruAntonescuKeysight. Do we need to update the paths as shown in "fixes include paths in the feeder after directory refactor"? Can you handle this in the YAML file instead? I would like to avoid changing the include path in the feeder. I assume someone uses it by adding the feeder directory as an include path. It also does not seem natural to have an include path starting with 'src,' at least for me.

@TaekyungHeo
Copy link
Contributor

TaekyungHeo commented May 15, 2024

This will break downstream tools because the include path has been updated.

@danmih-ixia
Copy link

since the files have been moved from "et_feeder" to "src/feeder", the "et_feeder/et_feeder.h" paths are currently broken so need to be fixed. I think we should use #include "et_feeder.h" and add the include path as parameter at compile time.

@TaekyungHeo
Copy link
Contributor

Yes, it makes sense. Please review and comment, @srinivas212

@srinivas212
Copy link
Contributor

@AlexandruAntonescuKeysight @danmih-ixia @TaekyungHeo - are there any other opens? The paths issue has been fixed.

@danmih-ixia
Copy link

@AlexandruAntonescuKeysight @danmih-ixia @TaekyungHeo - are there any other opens? The paths issue has been fixed.

I don't think there is anything pending. The path issue has been submitted now, please check.

@srinivas212 srinivas212 merged commit 74d086a into mlcommons:main May 17, 2024
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants