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

Add MATLAB code analyzer #3018

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Add MATLAB code analyzer #3018

merged 2 commits into from
Oct 31, 2024

Conversation

pepone
Copy link
Member

@pepone pepone commented Oct 30, 2024

This PR runs the MATLAB code analyzer on CI. I also include fixes for some of the issues reported by this analyzer.

@@ -97,54 +97,6 @@ runs:
shell: powershell
if: runner.os == 'Windows'

#
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the MATLAB setup to a separate action to be able to use it from both CI and MATLAB Analyzer workflows.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't both workflows use setup-dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was how setup-dependencies detect if MATLAB has to be installed:

 if: runner.os == 'Linux' && matrix.config == 'matlab'

There is no matrix config with the new job.

Copy link
Member

@externl externl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option is to add another bool input parameter to setup-dependences to indicate if Matlab should be installed. It would keep the setup in one spot.

@pepone pepone merged commit fb93213 into zeroc-ice:main Oct 31, 2024
18 checks passed
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.

3 participants