mosra / magnum

Lightweight and modular C++11 graphics middleware for games and data visualization
https://magnum.graphics/
Other
4.75k stars 439 forks source link

github actions for static code analsys? #590

Open jonesmz opened 1 year ago

jonesmz commented 1 year ago

I have several projects where I've added a github action for

If I create a pull request that added the same, would that be something you'd want to merge?

I ask because all of the current CI is done through appveyor, and I'm not familiar with it, and wouldn't be able to copy-paste (and then modify) my existing analyzers.yml file.

mosra commented 1 year ago

Hey, sorry for a late reply on this one.

There's AppVeyor, but there's also CircleCI that I use for all non-Windows builds. (See the full build matrix here.) Problem is that there's aready a lot of combinations, which take a lot of time (or a lot of credits on CircleCI) to go through, so I'd like to add new jobs only if they actually add some value. I'm not opposed to having some builds on GH Actions, relying on multiple providers is never a bad thing, although in my opinion the Actions UX is really terrible compared to CircleCI and AppVeyor. I would also need to write some more javascript to have it appear along the other two on the build status page.

Regarding IWYU, every time I tried it in the past, it was in direct conflict with the forward declaration headers Magnum is using -- i.e., suggesting to remove Trade/Trade.h include and replacing it with manually written forward declarations. Which is annoying and verbose (especially when typedefs are involved) and sometimes even impossible (such as when default template arguments are involved). I'm not aware of the tool fixing this since, and besides that every time I saw it in action it needed a lot of suppressions, "nolint" comments and manual fixups (such as suggesting it should use <utility> instead of <bits/stl_pair.h>, ugh). So that one I definitely don't want.

I remember running Clang Analyzer and LGTM in the past, but they only oever generated false positives for a perfectly valid code, so I eventually ditched them. I also have a stale PR for Clang Memory Sanitizer and UBSan, but that one got also stuck on many false positives that were just too annoying to suppress. In other words, if you do an analyzer build locally and it finds actual bugs (which, based on my past experience, is rather unlikely), I'll consider adding it to the CI build matrix. Otherwise not really.

I have to admit I don't know what CodeQL does. Same as with the analyzers, I can consider adding it if it proves to be useful. If it doesn't or generates far too many annoying / useless warnings like e.g. Clang Tidy, I don't want it.