Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

ci: Simplify CodeQL setup#27733

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ksunden merged 1 commit intomatplotlib:mainfromQuLogic:fix-codeql
Feb 2, 2024
Merged

Conversation

QuLogic
Copy link
Member

@QuLogicQuLogic commentedFeb 2, 2024
edited
Loading

PR summary

The workflow is now warning thatCODEQL_PYTHON should not be set, as it is no longer used. According to the message, we also don't need to install dependencies, so fold everything into the 'build-for-C++' step.

PR checklist

@QuLogicQuLogic added this to thev3.9.0 milestoneFeb 2, 2024
@QuLogicQuLogicforce-pushed thefix-codeql branch 2 times, most recently from278f46f toac93141CompareFebruary 2, 2024 02:20
@QuLogicQuLogic added CI: testingCI configuration and testing Maintenance labelsFeb 2, 2024
@QuLogic
Copy link
MemberAuthor

Looking at the results here, it appears that the C++ job is uploading results for 0 files. But so far, it seems like it'salways done that? I'm investigating older runs to see if this ever worked and/or when it broke.

@QuLogic
Copy link
MemberAuthor

I suspected that this changed with the Meson build, and looking at Code Scanning results, they were indeed all "fixed" (even the closed ones) as of Oct 4, 2023 by the merging of#26621.

@QuLogicQuLogic marked this pull request as draftFebruary 2, 2024 03:41
@github-advanced-security

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 onthis 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 outthe documentation.

@QuLogicQuLogicforce-pushed thefix-codeql branch 5 times, most recently from4ee402b tod670194CompareFebruary 2, 2024 06:36
@oscargus
Copy link
Member

Maybe you have already tried this in your previous attempts, but I was inspired to set up the same thing for one of my repositories when I saw this and with the default template it worked pretty simple. I basically skipped the autobuild and just installed it with pip. (And had to add a setup-python-step to be able to install using meson.)

However, I assume that the remaining problem is that not that many of the files are considered?

FWIW:https://github.com/apytypes/apytypes/blob/main/.github/workflows/codeql.yml

@QuLogic
Copy link
MemberAuthor

Oh, that's interesting; I would've thought that just building (as we used to do) would be equivalent, unless it's something to do with where it builds. I'll give that a try as well.

@QuLogicQuLogicforce-pushed thefix-codeql branch 4 times, most recently from68088d5 to867971cCompareFebruary 2, 2024 07:57
The workflow is now warning that `CODEQL_PYTHON` should not be set, asit is no longer used. According to the message, we also don't need toinstall dependencies, so fold everything into the 'build-for-C++' step.
@QuLogic
Copy link
MemberAuthor

Thanks, that worked and looks even simpler to me:

CodeQL scanned 79 out of 162 C files and 25 out of 60 C++ files in this job.

(Note: we get some extra from Qhull/FreeType/etc, so we get more C files than we actually have. We could maybe configure it to ignore those.)

But note that we don't need a new Python, just upgradingpip, as that fixes the automatic install ofmeson/ninja/pybind11.

@QuLogicQuLogic marked this pull request as ready for reviewFebruary 2, 2024 08:19
@dstansby
Copy link
Member

Looks good. One of the checks is complaining that:

Warning: Code scanning cannot determine the alerts introduced by this pull request, because 1 configuration present on refs/heads/main was not found

(https://github.com/matplotlib/matplotlib/pull/27733/checks?check_run_id=21143960748)

I'm guessing this will get fixed upon merging this into main?

@QuLogic
Copy link
MemberAuthor

Yes, I believe that's because we "lost" the C/C++ results onmain for a long while now.

Copy link
Member

@dstansbydstansby left a comment

Choose a reason for hiding this comment

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

@ksunden
Copy link
Member

All of the new alerts are in thesubprojects directory (i.e freetype/qhull code), so we can probably either mark them aswon't fix (like we have for some inttconv/otherextern things) or filter out the subprojects directory from results, as I don't think we are going to change that code.

@ksundenksunden merged commit26832df intomatplotlib:mainFeb 2, 2024
@QuLogicQuLogic deleted the fix-codeql branchFebruary 5, 2024 22:52
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ksundenksundenksunden approved these changes

@dstansbydstansbydstansby approved these changes

Assignees
No one assigned
Labels
CI: testingCI configuration and testingMaintenance
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

4 participants
@QuLogic@oscargus@dstansby@ksunden

[8]ページ先頭

©2009-2025 Movatter.jp