When reviewing sys/psa_crypto: Adding hmac hashing on psa_import_key and fix max hmac key size by Lukas-Luger · Pull Request #21297 · RIOT-OS/RIOT · GitHub I noticed there are spurious complaints about lines too long. They are lines which this PR does not actually touch, so this is code that was already in the repo. I will send a PR fixing this; but I want to make sure I’m running the right test. I found: RIOT/.github/workflows/static-test.yml at master · RIOT-OS/RIOT · GitHub which has a docker image, which I managed to run. (Bug: it doesn’t work if you are using a git worktree which is …/foobar, but maybe it would work if you put your worktree as a a subdir) But, I don’t think that actually is doing the line too long test. Where actually is that test?
Hi,
IMO gently nagging users to clean up the code base is actually a good thing. Since those a warning, contributors can choose to ignore them. But occasionally, sine they have to touch the file anyway, they add an unrelated commit cleaning up aomeone else’s mess.
I personally often do that, and so do e.g. Karl and Ben.
For that reason, I see that nagging about unrelated code in files a PR does toach as a tool to spread the work to clean up the code base.
I would vote for keeping it gently nagging.
If we could instead turn warnings to failures for lines actually touched by a PR, that would be IMHO extremely useful, provided there is an override in the code that would disable the check for a given region (e.g. a markdown table in a comment should rather be correct than at most 80 columns wide).
I’m not arguing for turning anything on or off, or whether we can limit ourselves to lines that are changed. I just want to run the same test… I found in that static-test.yml file, the docker run line that I thought would run the test, but it didn’t seem to. I’d rather just run the test over and over again until I get all the lines fixed. I can probably make up my own test… probably using cut -c100- or something like that… but I’d like to execute the test that the checker runs.
You should be able to run all static tests with
make static-test
The script for that you can find in dist/tools/ci/static_tests.sh.
I am not sure, if the line-too-long check is done by uncrustify or vera++ (or both) these days, but if you just want to run one of those specific linters, you should find be able to find it out there (or just comment out all other tests 
)
Thanks, “make static-test” is much simpler than a docker run. On my laptop, I needed: apt-get install pcregrep vera++ uncrustify codespell I didn’t figure out where to get “spatch” yet.
However, I had expected to see some complaints about the lines wider than 100 columns, and I did not. I don’t think
I ran, instead:
dyas-[RIOT/sys/psa_crypto](3.1.3) mcr 1368 %cut -c100- $(find . -name '*.[ch]' -print) | sed -e '/^$/d'
and saw that there definitely were still lines longer than 100. (the above doesn’t identify the files at all). So this wasn’t that helpful yet. But, i can grep for the strings, and fix them.
spatch stands for “semantic patch” and is provided by coccinelle.
Mh, there are two possibilities. Either you are in a branch different from master, then the scripts only looked at changed files or you are in master then everything should be checked. I haven’t used these tools locally in years. Maybe there was a regression since?
The list of files which are checked by the static tests is generated in the function changed_files in dist/tools/ci/changed_files.sh
Can doxygen @brief lines be wrapped? My historical impression was that they could not.
I don’t think so either. Apart from URLs that might be one of the reasons why this check is only a warning. Then again (as with commit message summaries who have similar restrictions), when they take up more than ~100 chars, are they really that “brief” .
I still haven’t found, or caused to be activated, the checker in question… once I understand it, maybe I can convince it to skip @brief lines. Maybe that line can be rewritten to the shorter.