Open mgcooper opened 1 month ago
The miss_hit badge should be deleted right? since pre commit is doing it?
The matlab test and coverage badge needs the extension to change from .yaml to .yml i.e. .github/workflows/matlab_test_and_coverage.yaml should be .github/workflows/matlab_test_and_coverage.yml
just fixed those 2: thanks for the heads up.
the issue though is that the URL of those badges is "hard coded" and will always point to the worfklows on this repo: definitely an advantage of using the cookie cutter approach over this template.
In matlab_test_and_coverage.yml:
* It might be necessary to make the matlab version consistent in the setup and run steps: * uses: matlab-actions/setup-matlab@v2.2.0 * uses: matlab-actions/run-command@v2.1.1 * (Not sure that's necessary, I thought it was why the tests failed on my fork but more likely due to other reasons)
Don't think this is necessary:
There is no v2.2.0 for run-command: https://github.com/matlab-actions/run-command/releases
I have sometimes had to update both of those, but if I see no failure I usually don't worry.
Also I rarely update manually github actions, I let dependabot open PR for me: https://github.com/Remi-Gau/template_matlab_analysis/blob/61d118504735583eb1a1e9e02a3eedd706ef4f2c/.github/dependabot.yml#L6
Other changes:
* I had to add .vscode/settings.json file to override my autoIndent settings otherwise the yml files were being indented with two spaces instead of four.
"[yml]": { "editor.tabSize": 4, "editor.insertSpaces": true, "editor.detectIndentation": false }
Will try to add those so that tools are most compatible.
I tried to fork and pull request these changes but as far as I can tell I cannot confirm if the fixes work unless they're done on the main branch (I think the matlab checkout feature only runs on the default branch). So I figured I'd just summarize them here.
In README:
In matlab_test_and_coverage.yml:
Other changes:
Unlike the matlab version thing, the indenting actually did cause the tests to fail, since the pre-commit settings enforce 4 spaces.
Feel free to take or leave any of these suggestions. I would have issued a PR but I spent a stupid amount of time trying to get the CI tests to run on a temporary fix branch of my fork and couldn't get them to go, so I gave up. Thanks for the nice template.