OpenHeartDevelopers / CemrgApp

An Interactive Medical Imaging Platform with Image Processing and Computer Vision Toolkits for Cardiovascular Research.
http://www.cemrgapp.com
BSD 3-Clause "New" or "Revised" License
18 stars 8 forks source link

Modernize GH Actions #78

Closed JostMigenda closed 5 months ago

JostMigenda commented 5 months ago
github-actions[bot] commented 5 months ago

:zap: Code Analysis Results :zap:

:red_circle: Cppcheck found 18 issues! Click here to see details.
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1813-L1818 ```diff !Line: 1813 - style: Redundant initialization for 'scalars'. The initialized value is overwritten before it is read. [redundantInitialization] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1806-L1811 ```diff !Line: 1806 - note: scalars is initialized ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Modules/CemrgAppModule/src/CemrgCommonUtils.cpp#L1813-L1818 ```diff !Line: 1813 - note: scalars is overwritten ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.h#L55-L60 ```diff !Line: 55 - style: The class 'AtrialFibresClipperView' does not have a constructor although it has private member variables. [noConstructor] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.h#L66-L71 ```diff !Line: 66 - style: The class 'AtrialFibresView' does not have a constructor although it has private member variables. [noConstructor] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L450-L455 ```diff !Line: 450 - style: Redundant initialization for 'radii'. The initialized value is overwritten before it is read. [redundantInitialization] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L449-L454 ```diff !Line: 449 - note: radii is initialized ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresClipperView.cpp#L450-L455 ```diff !Line: 450 - note: radii is overwritten ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresLandmarksView.h#L53-L58 ```diff !Line: 53 - style: The class 'AtrialFibresLandmarksView' does not have a constructor although it has private member variables. [noConstructor] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresLandmarksView.cpp#L392-L397 ```diff !Line: 392 - style: The scope of the variable 'distance' can be reduced. [variableScope] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresVisualiseView.h#L50-L55 ```diff !Line: 50 - style: The class 'AtrialFibresVisualiseView' does not have a constructor although it has private member variables. [noConstructor] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L1795-L1800 ```diff !Line: 1795 - style: Condition '!userInputAccepted' is always true [knownConditionTrueFalse] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L1793-L1798 ```diff !Line: 1793 - note: Assignment 'userInputAccepted=false', assigned value is 0 ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L1795-L1800 ```diff !Line: 1795 - note: Condition '!userInputAccepted' is always true ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L2340-L2345 ```diff !Line: 2340 - style: Variable 'segImage' is assigned a value that is never used. [unreadVariable] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L193-L198 ```diff !Line: 193 - style: Redundant initialization for 'reply1'. The initialized value is overwritten before it is read. [redundantInitialization] ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L190-L195 ```diff !Line: 190 - note: reply1 is initialized ```
https://github.com/OpenHeartDevelopers/CemrgApp/blob/466344b12a2a5da87e637e550c17d4795e3f6b3b/CemrgApp/Plugins/kcl.cemrgapp.atrialfibres/src/internal/AtrialFibresView.cpp#L193-L198 ```diff !Line: 193 - note: reply1 is overwritten ```

JostMigenda commented 5 months ago

The test action runs much farther now, which is nice to see; but the build step still fails with some references to gcc-6 and g++-6, even though I’ve eliminated those from all .yml files. So I’m not quite sure where that comes … unless it is in the precompiled Build folder that it’s downloading in the first step? 🤔

On a separate note: I notice that test.yml is currently set to run on: [push, pull_request]; which means it runs twice when I push a new commit to an open PR, which is wasteful. I’d therefore suggest changing it to only run on PRs?

alonsoJASL commented 5 months ago

The test action runs much farther now, which is nice to see; but the build step still fails with some references to gcc-6 and g++-6, even though I’ve eliminated those from all .yml files. So I’m not quite sure where that comes … unless it is in the precompiled Build folder that it’s downloading in the first step? 🤔

On a separate note: I notice that test.yml is currently set to run on: [push, pull_request]; which means it runs twice when I push a new commit to an open PR, which is wasteful. I’d therefore suggest changing it to only run on PRs?

Thank you Jost, these are great updates.

I agree that, as you mention, the references to older gcc versions would come from the prebuild folders downloaded. The solution would be to make these for the right versions. So, one in macOS, one in Ubuntu and one in Windows. These prebuild folders would be generated once we confirm the build instructions. So that's on my plate at the moment.

I'll accept the merge and we can pause till I provide the build folders. A question, we would need an intel mac and an arm mac prebuild folder, in case we manage arm, right?

JostMigenda commented 5 months ago

I'll accept the merge and we can pause till I provide the build folders. A question, we would need an intel mac and an arm mac prebuild folder, in case we manage arm, right?

Not necessarily. In principle, Qt versions that support ARM Macs also support cross builds or universal binaries; but how well that works in practice remains to be seen after we’ve upgraded Qt to a more recent version.