Skip to content
This repository was archived by the owner on Nov 27, 2024. It is now read-only.

ci: move away from differential shellcheck#65

Merged
sadlerap merged 2 commits intokonflux-workspaces:mainfrom
sadlerap:fix-shellcheck-ci
Apr 8, 2024
Merged

ci: move away from differential shellcheck#65
sadlerap merged 2 commits intokonflux-workspaces:mainfrom
sadlerap:fix-shellcheck-ci

Conversation

@sadlerap
Copy link
Member

@sadlerap sadlerap commented Apr 8, 2024

Differential shellcheck was ignoring errors in some cases, which was causing shellcheck warnings not to be picked up. Move away from it and use shellcheck-sarif as a SARIF provider.

sadlerap added 2 commits April 8, 2024 17:26
Differential shellcheck was ignoring errors in some cases, which was
causing shellcheck warnings not to be picked up.  Move away from it and
use [shellcheck-sarif][1] as a SARIF provider.

[1]: https://docs.rs/crate/shellcheck-sarif/latest

Signed-off-by: Andy Sadler <ansadler@redhat.com>
Signed-off-by: Andy Sadler <ansadler@redhat.com>
@sadlerap
Copy link
Member Author

sadlerap commented Apr 8, 2024

An example of shellcheck failures can be found here.

@sadlerap sadlerap requested a review from filariow April 8, 2024 22:33
Copy link
Member

@filariow filariow left a comment

Choose a reason for hiding this comment

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

LGTM

@sadlerap sadlerap merged commit ee816af into konflux-workspaces:main Apr 8, 2024
@sadlerap sadlerap deleted the fix-shellcheck-ci branch April 8, 2024 22:39
@jamacku
Copy link

jamacku commented Apr 25, 2024

@sadlerap Do you have any examples of when Differential ShellCheck ignored added defects? I would be interested, and @kdudka could be as well. Thank you.

@sadlerap
Copy link
Member Author

See #63 for an example of this in action.

Maybe it's because we weren't using a .shellcheckrc config, and now that we've added it as of #66 and #67, differential-shellcheck would work for us? I'm definitely willing to admit user error on my part 😄. I could give it another try; our current setup is definitely not the most efficient.

@jamacku
Copy link

jamacku commented Apr 26, 2024

Thank you for your feedback. You are right. It is the behavior described in #63 (review) because you have enabled some optional suggestions/checks locally that are not enabled by default.

Differential ShellCheck would report the same defects when you would enable them in .shellcheckrc.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants