Skip to content
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

CI: bump to ubuntu-22.04 #773

Closed
wants to merge 5 commits into from
Closed

CI: bump to ubuntu-22.04 #773

wants to merge 5 commits into from

Conversation

fabiangreffrath
Copy link
Owner

Let's see what breaks.

Let's see what breaks.
@rfomin
Copy link
Collaborator

rfomin commented Oct 22, 2022

I suggest to add --force to fix this warning:

Too many #ifdef configurations - cppcheck only checks 12 configurations. Use --force to check all configurations.

Then it seems to work more correctly.

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index f4f7ea7..04a4a70 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -47,7 +47,7 @@ jobs:
       - name: Run cppcheck
         shell: bash
         run: |
-          cppcheck --error-exitcode=1 -q -U_WIN32 -Isrc opl src setup textscreen
+          cppcheck --error-exitcode=1 -q -j4 --force -U_WIN32 -Isrc opl src setup textscreen
 
   win64_msys2:
     runs-on: windows-latest

@fabiangreffrath
Copy link
Owner Author

I suggest to add --force to fix this warning:

Oh, yes, sure. Could you please just add it?

@rfomin
Copy link
Collaborator

rfomin commented Oct 22, 2022

Oh, yes, sure. Could you please just add it?

OK. The UMAPINFO scanner errors are gone, but everything else is still there.

@fabiangreffrath
Copy link
Owner Author

Thanks! I think I'll have to report this string size alert in src/m_menu.c as a bug report to the cppcheck authors.

@fabiangreffrath
Copy link
Owner Author

ubuntu-22.04 will become ubuntu-latest as of December 1st:
actions/runner-images#6399

After this date, this PR will become obsolete and we can merge #771 to get rid of all new warnings at once.

@fabiangreffrath
Copy link
Owner Author

Thanks! I think I'll have to report this string size alert in src/m_menu.c as a bug report to the cppcheck authors.

I have just requested access (!) to the cppcheck issue tracker. Let's see where this leads us...

@rfomin
Copy link
Collaborator

rfomin commented Oct 26, 2022

I have just requested access (!) to the cppcheck issue tracker. Let's see where this leads us...

It seems ubuntu-22.04 has cppcheck-2.7 and there have been two releases since then: https://github.com/danmar/cppcheck/releases

Anyway I think there were no useful warnings from cppcheck ever. Maybe we should remove it?

@fabiangreffrath
Copy link
Owner Author

I have just requested access (!) to the cppcheck issue tracker. Let's see where this leads us...

It seems ubuntu-22.04 has cppcheck-2.7 and there have been two releases since then:

Right, but 2.9 in Debian emits the same warnings.

Anyway I think there were no useful warnings from cppcheck ever. Maybe we should remove it?

It was very helpful in the early days and is mostly meant to catch rough edges. I don't know when it started to go "super smart", but I think the few nit-picks that it unveils are easy enough to fix and improve code quality as a whole. The one odd issue that IMHO makes no sense will be reported upstream and meanwhile there is a trivial fix for it in the corresponding PR.

@fabiangreffrath
Copy link
Owner Author

Yep, this is indeed a bug in cppchecker that is now fixed:
danmar/cppcheck#4428

@fabiangreffrath
Copy link
Owner Author

ubuntu-22.04 is now ubuntu-latest

@fabiangreffrath fabiangreffrath deleted the ubuntu-22.04 branch December 28, 2022 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants