dracut icon indicating copy to clipboard operation
dracut copied to clipboard

Enable Differential ShellCheck

Open jamacku opened this issue 10 months ago • 7 comments

Differential ShellCheck is a GitHub action that performs differential ShellCheck scans on shell scripts changed via PR and reports results directly in PR.

It is able to produce reports in SARIF format. GitHub understands this format and is able to display it nicely as a PR comment, and on the Files Changed tab, please see below.

image

image

Checklist

  • [X] I have tested it locally
  • [X] I have reviewed and updated any documentation if relevant
  • [ ] I am providing new code and test(s) for it

This will resolve your issues with cleaning your codebase every time the new ShellCheck comes out: https://github.com/dracutdevs/dracut/pull/2501

Differential ShellCheck only reports defects directly related to the PR/commit. So you can focus on defects caused by your changes and deal with the existing defects later.

jamacku avatar Oct 03 '23 08:10 jamacku

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.

I can squash them If you prefer.

jamacku avatar Oct 30 '23 13:10 jamacku

Once you merge this pull request, the 'Security' tab will show more code scanning analysis results

@jamacku Have you had a chance to test this ? I run a brief test and at first run this will open about 300 code scanning issues based on the existing issues we have. Is this what is expected ?

What do you recommend after this happens ? Just leave the existing issues alone and focus on avoiding adding more issues ? Can you perhaps point to a project where we can see this in action ?

LaszloGombos avatar Nov 01 '23 02:11 LaszloGombos

@jamacku Have you had a chance to test this ? I run a brief test and at first run this will open about 300 code scanning issues based on the existing issues we have. Is this what is expected ?

You are right. There are 285 ShellCheck reports in the Security Dashboard. This is expected. It's for your awareness. You can review them and decide how to deal with them. The security dashboard allows you to set notes to each report and dismiss them.

What do you recommend after this happens ? Just leave the existing issues alone and focus on avoiding adding more issues ? Can you perhaps point to a project where we can see this in action ?

I usually leave the existing issues open and focus on not adding more. But the choice is yours.

The security dashboard is "private" to members of the organization, so you won't see the results of other projects that use differential shellcheck unless you are a member. Reports on PR can see anyone.

For example, the systemd project is keeping shell scripts free from ShellCheck defects - https://github.com/systemd/systemd/blob/main/.github/workflows/differential-shellcheck.yml

We also use it on our RHEL8 and RHEL9 dracut distribution git:

  • https://github.com/redhat-plumbers/dracut-rhel9/pull/63

It's beneficial to have differential ShellCheck scans because you can start linting your code base without dealing with current reports first (that saves time, allows you to check new incoming changes, and avoids potential regressions caused by fixing false positives).

jamacku avatar Nov 01 '23 12:11 jamacku

LGTM.

pvalena avatar Nov 01 '23 17:11 pvalena

@jamacku It seems the "Dracut Release Bot" pushed an extra commit to this PR. Can you help reverting the 4th commit on this PR ? Thanks !

LaszloGombos avatar Dec 01 '23 12:12 LaszloGombos

This issue is being marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. If this is still an issue in the latest release of Dracut and you would like to keep it open please comment on this issue within the next 7 days. Thank you for your contributions.

stale[bot] avatar Mar 13 '24 06:03 stale[bot]