Skip to content

Adds Github Code Scanning (CodeQL) #43

Merged
srinivas212 merged 5 commits intomlcommons:mainfrom
AlexandruAntonescuKeysight:main
May 17, 2024
Merged

Adds Github Code Scanning (CodeQL) #43
srinivas212 merged 5 commits intomlcommons:mainfrom
AlexandruAntonescuKeysight:main

Conversation

@AlexandruAntonescuKeysight
Copy link
Copy Markdown
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.

@AlexandruAntonescuKeysight AlexandruAntonescuKeysight requested a review from a team as a code owner April 30, 2024 15:10
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 30, 2024

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

@github-advanced-security
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
Contributor

TaekyungHeo commented May 15, 2024

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

@danmih-ixia
Copy link
Copy Markdown

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
Copy Markdown
Contributor

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

@srinivas212
Copy link
Copy Markdown
Contributor

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

@danmih-ixia
Copy link
Copy Markdown

@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
@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.

5 participants