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

ci(rust.yml/jobs/clippy): integrate with GitHub Code Scanning #417

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

DataM0del
Copy link
Contributor

@DataM0del DataM0del commented Dec 26, 2024

Description

See CCBlueX/LiquidBounce#4887, it's basically this but for detekt (because that's what LiquidBounce uses for linting their Kotlin part).

This PR pretty much just adds more QoL. Nothing really useful for anyone who isn't reviewing the code for PRs.
See https://docs.github.com/en/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning#about-third-party-code-scanning-tools
Also, you can probably open up the results.sarif file in any JetBrains IDE (e.g. IntelliJ IDEA) with Server-side Analysis and see all linting errors.

Testing

  • See if CI passes
  • After CI passes, intentionally make clippy trigger a warning
  • Observe the results and see if it works

Checklist

Things need to be done before this Pull Request can be merged.

  • Code is well-formatted and adheres to project style guidelines: cargo fmt
  • Code does not produce any clippy warnings: cargo clippy

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

@DataM0del
Copy link
Contributor Author

I forgot it to install clippy-sarif in the release mode linting job 💀

@DataM0del
Copy link
Contributor Author

Now let's trigger clippy.

pumpkin/src/main.rs Fixed Show fixed Hide fixed
pumpkin/src/main.rs Fixed Show fixed Hide fixed
pumpkin/src/main.rs Fixed Show fixed Hide fixed
pumpkin/src/main.rs Fixed Show fixed Hide fixed
pumpkin/src/main.rs Fixed Show fixed Hide fixed
pumpkin/src/main.rs Fixed Show fixed Hide fixed
@DataM0del
Copy link
Contributor Author

Now let's revert it!

@DataM0del
Copy link
Contributor Author

image

@DataM0del
Copy link
Contributor Author

It says fixed now.

@DataM0del DataM0del marked this pull request as ready for review December 26, 2024 15:23
@Snowiiii
Copy link
Member

So, What does this do ?

@DataM0del
Copy link
Contributor Author

So, What does this do ?

It adds a "review" that comments on each line that clippy screams at, instead of you having to go look at the CI logs for the clippy job.

@Snowiiii
Copy link
Member

I don't think this is really needed, I think its more annoying that actually useful. People also don't really have to go into the CI logs. They can just see what CI failed and then fix it locally

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.

2 participants