-
Notifications
You must be signed in to change notification settings - Fork 472
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
Replace LGTM static analysis with GitHub code scanning #3162
Conversation
OK, it should be ready now 😄 |
It looks like the static analysis isn't showing any alerts (we have fixed all the warnings?). Are the alerts configurable/are they set to the same settings as LGTM? It would be good to see what the alerts would look like and to make sure we aren't getting false negatives. |
This PR is now under review (see the table in the PR description). To help with the review process, please do not force push to the branch. |
A windows job failed again ... restarted ... |
I managed to trigger the static analysis warning in #3168, this is what it looks like: |
@pazner, it looks like the code scanning did not catch the memory leak you added in Edit: I now see that you also added a leak in |
Yes, I actually added two intentional leaks, one is exactly the same at the LGTM one, as well as a new one. It didn't complain about either one of them. I don't know if this can be changed with configuration/additional checks. |
Merged in |
I changed the settings in https://github.com/mfem/mfem/settings/security_analysis to the max levels. I am not aware of any additional settings at the GitHub. |
Originally motivated by LGTM.com shutting down, this PR replaces it with GitHub code scanning and adds a few more fixes in MFEM's GitHub actions:
After merging:
master
andnext
in https://github.com/mfem/mfem/settings/branchesPR Checklist
make style
.CHANGELOG
:CHANGELOG
to group with other related features?INSTALL
:make
orcmake
have a new target?.github
.appveyor.yml
.gitignore
:make distclean; git status
shows any files that were generated from the source by the project (not an IDE) but we don't want to track in the repository.examples/makefile
:SEQ_EXAMPLES
andPAR_EXAMPLES
variables.clean
target..gitignore
file.examples/CMakeLists.txt
:ALL_EXE_SRCS
variable.THIS_TEST_OPTIONS
is set correctly for the new example.doc/CodeDocumentation.dox
.examples/pumi
), list it indoc/CodeDocumentation.conf.in
src/examples.md
.src/examples.md
andsrc/img
.examples.md
, list the example under the appropriate categories, add new categories if necessary.features.md
.makefile
andmakefile
in corresponding miniapp directory..gitignore
file.CMakeLists.txt
file in theminiapps
directory, if the new miniapp is in a new directory.CMakeLists.txt
file in the new miniapp directory.doc/CodeDocumentation.dox
miniapps/nurbs
), add it toMINIAPP_SUBDIRS
in themakefile
.miniapps/nurbs
), list it indoc/CodeDocumentation.conf.in
src/meshing.md
andsrc/electromagnetics.md
files.src/examples.md
andsrc/img
.features.md
.mfem/web
repo.README
(rare).doc/CodeDocumentation.dox
(rare).make unittest
to make sure all unit tests pass.tests/scripts
.