Skip to content

[STAL-2057] Hardcode include testing rules in CLI #359

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

Merged
merged 3 commits into from
May 8, 2024

Conversation

modernplumbing
Copy link
Contributor

@modernplumbing modernplumbing commented May 8, 2024

What problem are you trying to solve?

When using the Analyzer via the CLI, all rules, including testing rules, should be included.

What is your solution?

By hard-coding that all rules, including testing rules, should be pulled from the API, we remove the complexity associated with parameterizing whether or not testing rules should be included. The con of including testing rules by default is virtually only that additional results are in the resulting SARIF file. When the SARIF file gets sent to Datadog, violations from testing rules are hidden in the UI by default (but is easily accessed via a toggle). The pro of including testing rules by default is that it reduces the number of places a user needs to specify that testing rules should be included.

Alternatives considered

  1. Making the query param exclude_testing_rules instead of include_testing_rules. The con of this is that we'd need to change the semantics of this feature in many places.
  2. Update Github Action to specify that testing rules should be included, and update usage of the GHA and the CLI in many places. This choice would also result in a more manual work, and also no guarantee that all downstream consumers would pick up this change.

What the reviewer should know

@modernplumbing modernplumbing marked this pull request as ready for review May 8, 2024 19:54
@modernplumbing modernplumbing merged commit 6b2f320 into main May 8, 2024
42 of 46 checks passed
@jasonforal jasonforal deleted the celia.yuen/hardcode-include-testing-true branch November 8, 2024 09:37
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