-
Notifications
You must be signed in to change notification settings - Fork 363
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
Refactoring: CppCheck rules cleanup #1247
Comments
@Bertk not sure? If someone wanna use an older copcheck version he needs the old rules. |
@guwirth Yes, we had this discussion before. I think an old version of a static analysis tool or checker is pretty useless and we have now ~115 legacy rules.
|
@Bertk would make a bigger cut when we start over with SQ 6.7 LTS ++ support .
|
The external tools are unrelated to the release cycle of SQ. IMHO it would be better to define a simple policy here (which could be violated if necessary but t give people an idea...):
|
@amai2012 currently we are supporting the LTS versions and if possible also the versions in-between. I say "if possible" because sometimes this is not possible because of SQ API changes. All analysis sensors have also a property for customer rules ( In case of an update to a new LTS version we typically also try to clean-up. This based typically on the feedback we get and less on fixed regulations. |
@guwirth : My comment was referring to external tools. So you have to decide which versions of cppcheck/compilers/etc. you want to decide. So given the date for the next release you can determine which version of cppcheck you want to support. |
Proposal is to support 1.80+ only and remove older messages |
I disagree to the suggestion. My reason is the integration of cppcheck into the CI chain of large enterprises. The most of them use the long-term-support (LTS) versions of some commercially supported Linux distribution and are limited to the packages of corresponding repositories. After a brief look at https://pkgs.org/download/cppcheck you'll see, that the LTS versions have very old cppcheck versions.
The limitation to the v 1.80+ would not harm private developers and less conservative [small] enterprises, but it could be too radical for the large ones. Alternative proposalWe could freeze the current cppcheck rules-set (repository) and rename it to
After cppcheck v 1.86 will be released we'll merge Pros: 1) fine grained rule-sets without performance impact 2) new rule repositories are generated automatically, no manual post-processing, merging etc IMHO the alternative solution has more cons than pros but it's still better the the original proposal. |
I am not aware of open source quality tools that feature a LTS (do they exist?) - so keeping the server on an LTS is reasonable, skipping the new tools usually is not. At least unless the current version of the tools changed their scope and removed support for legacy environments (C89, C++03, Java 6, etc.) In case one would take care of LTS distributions one should cherry-pick the latest, e.g. ubuntu 18 which in turn would suggest cppcheck 1.82. |
@amai2012 if somebody decides to install a LTS Linux, such installation won't be upgraded for a long period of time. That's because the vendor back-ports all security patches (CVEs), not only for the kernel itself, but even for affected packages from the corresponding Linux repository. E.g. Ubuntu provides LTS support for 5 years. New LTS releases are published every 2 years (2018 -> 18.04, 2016 -> 16.04 etc). That makes Ubuntu 16.04. with cppcheck 1.72 to a pretty new release. |
The question is then if it is worth to spend the effort to cleanup? Maybe we should close this issues? I don’t wanna maintain two rule-sets. We could also mark older rules as deprecated? Or add a tag? |
Many CppCheck rules in the file cxx-sensors\src\main\resources\cppcheck.xml are obsolete (~115) and shall be removed - see attached list compared with errorlist from CppCheck V1.81 (diff.txt).
There are some interim and obsolete rule names beside rules name with typos which are corrected now.
The text was updated successfully, but these errors were encountered: