Skip to content

Add static code analysis on commit and fix CodeQL errors #1507

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.

Sign up for GitHub

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

Jump to bottom
Merged
robertwu1 merged 14 commits into from
Mar 23, 2022
Merged

Add static code analysis on commit and fix CodeQL errors #1507

robertwu1 merged 14 commits into from
Mar 23, 2022

Conversation

@robertwu1
Copy link
Collaborator

@robertwu1 robertwu1 commented Mar 8, 2022

Create an action to run static code analysis on demand.

After this is created, a separate PR will be created to fix linker issues described in #1506

@robertwu1
Create codeql.yml for static code analysis
5b2dff0 
Create an action to run static code analysis on demand.
@robertwu1 robertwu1 requested a review from philburk March 8, 2022 22:09
@robertwu1
Copy link
Collaborator Author

robertwu1 commented Mar 8, 2022

Seems like there are issues in the autobuilder. github/codeql-action#824

@ann0see
Copy link

ann0see commented Mar 10, 2022 •
edited
Loading

What happens if you just use a normal build process instead of the auto-build provided by codeql (probably you’re already doing that though)

@robertwu1
Copy link
Collaborator Author

robertwu1 commented Mar 10, 2022

What happens if you just use a normal build process instead of the auto-build provided by codeql (probably you’re already doing that though)

Changing the build to something normal seems to work fine. Thanks! I will address the issues next week

@robertwu1 robertwu1 changed the title Create codeql.yml for static code analysis Add static code analysis on commit Mar 10, 2022
@ann0see
Copy link

ann0see commented Mar 11, 2022

Ah, now similar issues show up in the CI: https://github.com/google/oboe/runs/5504278249

@ann0see
Copy link

ann0see commented Mar 15, 2022

Great! Thanks for fixing these errors

@ann0see
Copy link

ann0see commented Mar 15, 2022

Do you plan to add codeQl to your repo or just fix the overflow warnings? I think splitting the fixes in a separate PR would make sense such that you can decide if CodeQL should or shouldn’t be added.

@robertwu1 robertwu1 changed the title Add static code analysis on commit Add static code analysis on commit and fix CodeQL errors Mar 15, 2022
@robertwu1
Copy link
Collaborator Author

robertwu1 commented Mar 15, 2022

Is there a downside of adding CodeQl to the CI besides the checks taking a bit longer?

@ann0see
Copy link

ann0see commented Mar 15, 2022

Not really.

philburk
philburk suggested changes Mar 17, 2022
View reviewed changes
@robertwu1 robertwu1 requested a review from philburk March 17, 2022 19:14
@robertwu1
Copy link
Collaborator Author

robertwu1 commented Mar 23, 2022

Fixes #1506

@robertwu1 robertwu1 merged commit 2d4797a into main Mar 23, 2022
@ann0see
Copy link

ann0see commented Mar 23, 2022

Great thanks for fixing and merging!

@ann0see ann0see mentioned this pull request Mar 23, 2022
Closed
@robertwu1 robertwu1 deleted the robertwu/addcodeql branch April 6, 2022 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@philburk philburk philburk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@robertwu1 robertwu1

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@robertwu1 @ann0see @philburk