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

README badges #45

Open
mgcooper opened this issue Jul 10, 2024 · 3 comments
Open

README badges #45

mgcooper opened this issue Jul 10, 2024 · 3 comments

Comments

@mgcooper
Copy link

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:

  • 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

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/[email protected]
    • uses: matlab-actions/[email protected]
    • (Not sure that's necessary, I thought it was why the tests failed on my fork but more likely due to other reasons)

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
    }
  • I also had to add a .prettierrc file to the top-level to override the yml indenting. Lots of people use prettier so this may be worth adding to the repo. This is the entire contents of the file:
{
  "tabWidth": 4,
  "useTabs": false
}

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.

@Remi-Gau
Copy link
Owner

  • 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.

@Remi-Gau
Copy link
Owner

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/[email protected]
  * uses: matlab-actions/[email protected]
  * (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:

- package-ecosystem: github-actions

@Remi-Gau
Copy link
Owner

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.

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

No branches or pull requests

2 participants