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 MegaLinter for Linting and Fixing #5

Merged
merged 127 commits into from
Dec 6, 2024
Merged

Conversation

ScottGibb
Copy link
Collaborator

@ScottGibb ScottGibb commented Nov 25, 2024

Summary

This PR aims to add Linting to the project via MegaLinter(https://github.com/oxsecurity/megalinter). The reason we want to use MegaLinter as it has a lot of out of the box setup and runs in docker so can be used cross platform and allows everyone to use the same linters.

Locally Running the Linter

npx mega-linter-runner

This command will also auto format and fix various files as they are configured via the .mega-linter.yml file.

Linting Workflow

This PR aims to fix both the codebase files and add the linting workflow.

The linting workflow at present works as follows:

On a Pull Request:

  • Checks all Code using the Linting process, fails on Errors
  • If code has fixable errors such as formatting or easy linting errors, a new branch will be created and a second pull request opened with these fixes.

The Linting workflow will pass when all checked files have zero errors.

Due to some limitations in cargo clippy tool and rustfmt in MegaLinter an extra stage workflow was added to ensure full coverage.

Tasks

  • Add Auto PR formatting - Couldn't be added due to it not working correctly and more hassle than its worse.
  • Make Sure Clippy is looking at entire project - Posted on Discussion to find out more (Rust Clippy Linter Settings: How to run Clippy and fmt on different crates and example directories with manifest files oxsecurity/megalinter#4318)
  • Make sure Markdownlint is looking at entire project (Confirmed and will auto fix things if the .markdownlint.json is present )
  • Create Ticket for updating MegaLinter Configuration when it uses cargo fmt and can scan the esp32c3 example
  • Confirm with MegaLinter Devs about PR workflow
  • Resolve Line Length Standard
  • PAT Token for actionlint fixes
  • Add Markdownlint and yamllint line count settings to main`.mega-linter.yaml' settings

@ScottGibb
Copy link
Collaborator Author

Line Length has taken the standard of no new line characters except for paragraphs and relying on the developers/githubs soft line wraps for display

@ScottGibb
Copy link
Collaborator Author

Ive removed the dead workflow code and think the best way to move forward is to leave an issue on the repo linking to my question on the megalinter question. That way when there is a response we can update the repo workflow with an updated flow or append the example in megalinter.

@ScottGibb
Copy link
Collaborator Author

ScottGibb commented Nov 29, 2024

For removing the markdownlint.json we have some issues. I cant seem to override the default config without providing our own.

The default is this

{
  "MD004": false,
  "MD007": {
    "indent": 2
  },
  "MD013": {
    "line_length": 400
  },
  "MD026": {
    "punctuation": ".,;:!。,;:"
  },
  "MD029": false,
  "MD033": false,
  "MD036": false,
  "blank_lines": false
}

Personally I dont like the default as its not the actual default that markdownlint provides.

Ive tried overriding it through the .mega-linter.yaml by using command arguments sadly this doesnt work.

Im opting for us using our own config at the root level

This comment has been minimized.

@ScottGibb ScottGibb marked this pull request as ready for review November 29, 2024 14:54
@ScottGibb ScottGibb added the enhancement New feature or request label Nov 29, 2024
src/registers.rs Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Dec 5, 2024

MegaLinter has automatically applied linters fixes on this PR.
Please review the changes and merge if they are correct.
PR: #16

Copy link
Collaborator

@jamessizeland jamessizeland left a comment

Choose a reason for hiding this comment

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

Excellent work

@ScottGibb ScottGibb merged commit 7fc5807 into main Dec 6, 2024
3 checks passed
@ScottGibb ScottGibb deleted the add-static-analysis branch December 6, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants